From: Paul Floyd Date: Mon, 29 Jan 2024 07:03:52 +0000 (+0100) Subject: FreeBSD ioctl: add handling for FIODGNAME X-Git-Tag: VALGRIND_3_23_0~194 X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=d0c17b5431e5448c73f99322afb5aac442c2ee70;p=thirdparty%2Fvalgrind.git FreeBSD ioctl: add handling for FIODGNAME Up til now FreeBSD ioctl coverage was conspicuous by its absence relying entirely on the generic wrapper. Time to start improving that. Example provided by Kyle Evans. --- diff --git a/.gitignore b/.gitignore index 88a6522605..7ad5cf912b 100644 --- a/.gitignore +++ b/.gitignore @@ -1395,6 +1395,7 @@ /memcheck/tests/freebsd/linkat /memcheck/tests/freebsd/memalign /memcheck/tests/freebsd/misc +/memcheck/tests/freebsd/openpty /memcheck/tests/freebsd/pdfork_pdkill /memcheck/tests/freebsd/realpathat /memcheck/tests/freebsd/revoke diff --git a/coregrind/m_syswrap/syswrap-freebsd.c b/coregrind/m_syswrap/syswrap-freebsd.c index bfb1cf876e..d7e63ef4bb 100644 --- a/coregrind/m_syswrap/syswrap-freebsd.c +++ b/coregrind/m_syswrap/syswrap-freebsd.c @@ -974,53 +974,23 @@ PRE(sys_setlogin) // int ioctl(int fd, unsigned long request, ...); PRE(sys_ioctl) { - UInt dir = _VKI_IOC_DIR(ARG2); - UInt size = _VKI_IOC_SIZE(ARG2); *flags |= SfMayBlock; // @todo PJF presumably the presence of ARG3 depends on ARG2 PRINT("sys_ioctl ( %" FMT_REGWORD "u, 0x%" FMT_REGWORD "x, %#" FMT_REGWORD "x )",ARG1,ARG2,ARG3); PRE_REG_READ3(int, "ioctl", int, fd, unsigned long, request, unsigned long, arg); - /* On FreeBSD, ALL ioctl's are IOR/IOW encoded. Just use the default decoder */ - if (SimHintiS(SimHint_lax_ioctls, VG_(clo_sim_hints))) { - /* - * Be very lax about ioctl handling; the only - * assumption is that the size is correct. Doesn't - * require the full buffer to be initialized when - * writing. Without this, using some device - * drivers with a large number of strange ioctl - * commands becomes very tiresome. - */ - } else if (dir == _VKI_IOC_NONE && size > 0) { - static UWord unknown_ioctl[10]; - static Int moans = sizeof(unknown_ioctl) / sizeof(unknown_ioctl[0]); - if (moans > 0 && !VG_(clo_xml)) { - /* Check if have not already moaned for this request. */ - UInt i; - for (i = 0; i < sizeof(unknown_ioctl)/sizeof(unknown_ioctl[0]); i++) { - if (unknown_ioctl[i] == ARG2) { - break; - } - if (unknown_ioctl[i] == 0) { - unknown_ioctl[i] = ARG2; - moans--; - VG_(umsg)("Warning: noted but unhandled ioctl 0x%lx" - " with no direction hints.\n", ARG2); - VG_(umsg)(" This could cause spurious value errors to appear.\n"); - VG_(umsg)(" See README_MISSING_SYSCALL_OR_IOCTL for " - "guidance on writing a proper wrapper.\n" ); - return; - } - } - } - } else { - if ((dir & _VKI_IOC_WRITE) && size > 0) { - PRE_MEM_READ( "ioctl(generic)", ARG3, size); - } - if ((dir & _VKI_IOC_READ) && size > 0) { - PRE_MEM_WRITE( "ioctl(generic)", ARG3, size); - } + switch (ARG2) { + case VKI_FIODGNAME: { + struct vki_fiodgname_arg* data = (struct vki_fiodgname_arg*)(Addr)ARG3; + PRE_FIELD_READ("ioctl(FIODGNAME).len", data->len); + PRE_FIELD_READ("ioctl(FIODGNAME).buf", data->buf); + PRE_MEM_WRITE("ioctl(FIODGNAME).buf", (Addr)data->buf, data->len); + break; + } + default: + ML_(PRE_unknown_ioctl)(tid, ARG2, ARG3); + break; } // The block below is from Ryan Stone @@ -1087,12 +1057,15 @@ PRE(sys_ioctl) POST(sys_ioctl) { - UInt dir = _VKI_IOC_DIR(ARG2); - UInt size = _VKI_IOC_SIZE(ARG2); - vg_assert(SUCCESS); - if (size > 0 && (dir & _VKI_IOC_READ) - && RES == 0 && ARG3 != (Addr)NULL) { - POST_MEM_WRITE(ARG3, size); + switch (ARG2) { + case VKI_FIODGNAME: { + struct vki_fiodgname_arg* data = (struct vki_fiodgname_arg*)(Addr)ARG3; + POST_MEM_WRITE((Addr)data->buf, data->len); + break; + } + default: + ML_(POST_unknown_ioctl)(tid, RES, ARG2, ARG3); + break; } #if 0 diff --git a/include/vki/vki-freebsd.h b/include/vki/vki-freebsd.h index d19c8de018..abbdf49c27 100644 --- a/include/vki/vki-freebsd.h +++ b/include/vki/vki-freebsd.h @@ -1086,6 +1086,11 @@ extern unsigned int __vki_invalid_size_argument_for_IOC; #define VKI_FIOASYNC _VKI_IOW('f', 125, int) #define VKI_FIOSETOWN _VKI_IOW('f', 124, int) #define VKI_FIOGETOWN _VKI_IOW('f', 123, int) +struct vki_fiodgname_arg { + int len; + void *buf; +}; +#define VKI_FIODGNAME _VKI_IOW('f', 120, struct vki_fiodgname_arg) /* get dev. name */ // See syswrap-freebsd.c PRE/POST(sys_ioctl) #if 0 diff --git a/memcheck/tests/freebsd/Makefile.am b/memcheck/tests/freebsd/Makefile.am index 466799cefd..d9efd74a99 100644 --- a/memcheck/tests/freebsd/Makefile.am +++ b/memcheck/tests/freebsd/Makefile.am @@ -76,6 +76,8 @@ EXTRA_DIST = \ memalign_supp.supp \ misc.vgtest \ misc.stderr.exp \ + openpty.vgtest \ + openpty.stderr.exp \ pdfork_pdkill.vgtest \ pdfork_pdkill.stderr.exp \ realpathat.vgtest \ @@ -144,6 +146,7 @@ check_PROGRAMS = \ file_locking_wait6 \ get_set_context get_set_login getfh \ kqueue linkat memalign misc \ + openpty \ pdfork_pdkill getfsstat inlinfo inlinfo_nested.so \ revoke scalar \ scalar_fork scalar_thr_exit scalar_abort2 scalar_pdfork \ @@ -200,3 +203,5 @@ errno_aligned_allocs_CFLAGS = ${AM_CFLAGS} @FLAG_W_NO_NON_POWER_OF_TWO_ALIGNMENT extattr_CFLAGS = ${AM_CFLAGS} @FLAG_W_NO_UNUSED_BUT_SET_VARIABLE@ memalign_CFLAGS = ${AM_CFLAGS} @FLAG_W_NO_NON_POWER_OF_TWO_ALIGNMENT@ + +openpty_LDFLAGS = ${AM_LDFLAGS} -lutil diff --git a/memcheck/tests/freebsd/openpty.c b/memcheck/tests/freebsd/openpty.c new file mode 100644 index 0000000000..bfba8ce578 --- /dev/null +++ b/memcheck/tests/freebsd/openpty.c @@ -0,0 +1,51 @@ +/* Build with cc -lutil valgrind-ptsname.c */ + +#include +#include + +#include +#include + +#include + +/* + * ==18203== Syscall param ioctl(generic) points to uninitialised byte(s) + * ==18203== at 0x49AAE7A: ioctl (in /lib/libc.so.7) + * ==18203== by 0x490C116: fdevname_r (in /lib/libc.so.7) + * ==18203== by 0x49E1567: ptsname (in /lib/libc.so.7) + * ==18203== by 0x486B5E2: openpty (in /lib/libutil.so.9) + * ==18203== by 0x2016E6: main (in /tmp/a.out) + * ==18203== Address 0x1ffc000a74 is on thread 1's stack + * ==18203== in frame #1, created by fdevname_r (???:) + */ + +#if 0 +/* Relevant bit from lib/libc/gen/fdevname.c */ +char * +fdevname_r(int fd, char *buf, int len) +{ + struct fiodgname_arg fgn; + + /* + * buf here points to a `static` buffer in ptsname.c, these are the only + * two members of fiodgname_arg. + */ + fgn.buf = buf; + fgn.len = len; + + if (_ioctl(fd, FIODGNAME, &fgn) == -1) + return (NULL); + return (buf); +} +#endif + +int +main() +{ + int master, slave; + + (void)openpty(&master, &slave, NULL, NULL, NULL); + + return (0); +} + diff --git a/memcheck/tests/freebsd/openpty.stderr.exp b/memcheck/tests/freebsd/openpty.stderr.exp new file mode 100644 index 0000000000..e69de29bb2 diff --git a/memcheck/tests/freebsd/openpty.vgtest b/memcheck/tests/freebsd/openpty.vgtest new file mode 100644 index 0000000000..0568f0a501 --- /dev/null +++ b/memcheck/tests/freebsd/openpty.vgtest @@ -0,0 +1,2 @@ +prog: openpty +vgopts: -q