From c922a7201dc63fc9f3e9fd04e67a25cdafcc138e Mon Sep 17 00:00:00 2001 From: Nicholas Nethercote Date: Wed, 12 Aug 2009 02:30:20 +0000 Subject: [PATCH] Add a crappy wrapper for access_extended(), one of the more ridiculous syscalls I've had the displeasure of encountering. Due to its ridiculousness, the wrapper misses a PRE_MEM_WRITE check and so can result in false positives. The POST_MEM_WRITE update is present, though, so it shouldn't cause subsequent problems. Fixes bug 200760. git-svn-id: svn://svn.valgrind.org/valgrind/trunk@10786 --- coregrind/m_syswrap/priv_syswrap-darwin.h | 2 +- coregrind/m_syswrap/syswrap-darwin.c | 94 +++++++++++++++++++++-- include/vki/vki-darwin.h | 10 +++ memcheck/tests/darwin/scalar.c | 7 +- memcheck/tests/darwin/scalar.stderr.exp | 19 +++++ 5 files changed, 125 insertions(+), 7 deletions(-) diff --git a/coregrind/m_syswrap/priv_syswrap-darwin.h b/coregrind/m_syswrap/priv_syswrap-darwin.h index 732a05584e..776f5e01e8 100644 --- a/coregrind/m_syswrap/priv_syswrap-darwin.h +++ b/coregrind/m_syswrap/priv_syswrap-darwin.h @@ -341,7 +341,7 @@ DECL_TEMPLATE(darwin, lstat_extended); // 280 DECL_TEMPLATE(darwin, fstat_extended); // 281 DECL_TEMPLATE(darwin, chmod_extended); // 282 DECL_TEMPLATE(darwin, fchmod_extended); // 283 -// NYI access_extended 284 +DECL_TEMPLATE(darwin, access_extended); // 284 DECL_TEMPLATE(darwin, settid); // 285 // NYI gettid 286 // NYI setsgroups 287 diff --git a/coregrind/m_syswrap/syswrap-darwin.c b/coregrind/m_syswrap/syswrap-darwin.c index bb1db2c528..23dacb892d 100644 --- a/coregrind/m_syswrap/syswrap-darwin.c +++ b/coregrind/m_syswrap/syswrap-darwin.c @@ -2015,7 +2015,8 @@ PRE(fchmod_extended) vki_mode_t, mode, void* /*really,user_addr_t*/, xsecurity); /* DDD: relative to the xnu sources (kauth_copyinfilesec), this - is just way wrong. */ + is just way wrong. [The trouble is with the size, which depends on a + non-trival kernel computation] */ PRE_MEM_READ( "fchmod_extended(xsecurity)", ARG5, sizeof(struct kauth_filesec) ); } @@ -2034,12 +2035,95 @@ PRE(chmod_extended) void* /*really,user_addr_t*/, xsecurity); PRE_MEM_RASCIIZ("chmod_extended(path)", ARG1); /* DDD: relative to the xnu sources (kauth_copyinfilesec), this - is just way wrong. */ + is just way wrong. [The trouble is with the size, which depends on a + non-trival kernel computation] */ PRE_MEM_READ( "chmod_extended(xsecurity)", ARG5, sizeof(struct kauth_filesec) ); } +// This is a ridiculous syscall. Specifically, the 'entries' argument points +// to a buffer that contains one or more 'accessx_descriptor' structs followed +// by one or more strings. Each accessx_descriptor contains a field, +// 'ad_name_offset', which points to one of the strings (or it can contain +// zero which means "reuse the string from the previous accessx_descriptor"). +// +// What's really ridiculous is that we are only given the size of the overall +// buffer, not the number of accessx_descriptors, nor the number of strings. +// The kernel determines the number of accessx_descriptors by walking through +// them one by one, checking that the ad_name_offset points within the buffer, +// past the current point (or that it's a zero, unless its the first +// descriptor); if so, we assume that this really is an accessx_descriptor, +// if not, we assume we've hit the strings section. Gah. +// +// This affects us here because number of entries in the 'results' buffer is +// determined by the number of accessx_descriptors. So we have to know that +// number in order to do PRE_MEM_WRITE/POST_MEM_WRITE of 'results'. In +// practice, we skip the PRE_MEM_WRITE step because it's easier to do the +// computation after the syscall has succeeded, because the kernel will have +// checked for all the zillion different ways this syscall can fail, and we'll +// know we have a well-formed 'entries' buffer. This means we might miss some +// uses of unaddressable memory but oh well. +// +PRE(access_extended) +{ + PRINT("access_extended( %#lx(%s), %lu, %#lx, %lu )", + ARG1, (char *)ARG1, ARG2, ARG3, ARG4); + PRE_REG_READ4(int, "access_extended", void *, entries, vki_size_t, size, + vki_errno_t *, results, vki_uid_t *, uid); + PRE_MEM_READ("access_extended(entries)", ARG1, ARG2 ); + + // XXX: as mentioned above, this check is too hard to do before the + // syscall. + //PRE_MEM_WRITE("access_extended(results)", ARG3, ??? ); +} +POST(access_extended) +{ + // 'n_descs' is the number of descriptors we think are in the buffer. We + // start with the maximum possible value, which occurs if we have the + // shortest possible string section. The shortest string section allowed + // consists of a single one-char string (plus the NUL char). Hence the + // '2'. + struct vki_accessx_descriptor* entries = (struct vki_accessx_descriptor*)ARG2; + SizeT size = ARG2; + Int n_descs = (size - 2) / sizeof(struct accessx_descriptor); + Int i = 0; // Current position in the descriptors section array. + Int u; // Upper bound on the length of the descriptors array + // (recomputed each time around the loop) + vg_assert(n_descs > 0); + + // Step through the descriptors, lowering 'n_descs' until we know we've + // reached the string section. + while (True) + { + // If we're past our estimate, we must be one past the end of the + // descriptors section (ie. at the start of the string section). Stop. + if (i >= n_descs) + break; + + // Get the array index for the string, but pretend momentarily that it + // is actually another accessx_descriptor. That gives us an upper bound + // on the length of the descriptors section. (Unless the index is zero, + // in which case we have no new info.) + u = entries[i].ad_name_offset / sizeof(struct vki_accessx_descriptor); + if (u == 0) { + vg_assert(i != 0); + continue; + } + + // If the upper bound is below our current estimate, revise that + // estimate downwards. + if (u < n_descs) + n_descs = u; + } + + // Sanity check. + vg_assert(n_descs <= VKI_ACCESSX_MAX_DESCRIPTORS); + + POST_MEM_WRITE( ARG3, n_descs * sizeof(vki_errno_t) ); +} + + PRE(chflags) { PRINT("chflags ( %#lx(%s), %lu )", ARG1, (char *)ARG1, ARG2); @@ -7477,9 +7561,9 @@ const SyscallTableEntry ML_(syscall_table)[] = { MACXY(__NR_stat_extended, stat_extended), MACXY(__NR_lstat_extended, lstat_extended), // 280 MACXY(__NR_fstat_extended, fstat_extended), - MACX_(__NR_chmod_extended, chmod_extended), - MACX_(__NR_fchmod_extended, fchmod_extended), -// _____(__NR_access_extended), + MACX_(__NR_chmod_extended, chmod_extended), + MACX_(__NR_fchmod_extended,fchmod_extended), + MACXY(__NR_access_extended,access_extended), MACX_(__NR_settid, settid), // _____(__NR_gettid), // _____(__NR_setsgroups), diff --git a/include/vki/vki-darwin.h b/include/vki/vki-darwin.h index 9032fdcf93..b5868879a9 100644 --- a/include/vki/vki-darwin.h +++ b/include/vki/vki-darwin.h @@ -728,6 +728,8 @@ typedef #define VKI_W_OK W_OK #define VKI_R_OK R_OK +#define vki_accessx_descriptor accessx_descriptor +#define VKI_ACCESSX_MAX_DESCRIPTORS ACCESSX_MAX_DESCRIPTORS #include @@ -1025,4 +1027,12 @@ struct ByteRangeLockPB2 #define vki_aiocb aiocb + +// XXX: for some reason when I #include I get a syntax +// error. Hmm. So just define things ourselves. +//#include + +//#define vki_errno_t +typedef int vki_errno_t; + #endif diff --git a/memcheck/tests/darwin/scalar.c b/memcheck/tests/darwin/scalar.c index 097a234b12..6834e2810f 100644 --- a/memcheck/tests/darwin/scalar.c +++ b/memcheck/tests/darwin/scalar.c @@ -510,7 +510,12 @@ int main(void) // __NR_chmod_extended 282 // __NR_fchmod_extended 283 - // __NR_access_extended 284 + + // XXX: we don't check the 'results' (too hard, see the wrapper code). If + // we did, it would be 2m. + GO(__NR_access_extended, 284, "4s 1m"); + SY(__NR_access_extended, x0, x0+1, x0, x0); FAIL; + // __NR_settid 285 // __NR_gettid 286 // __NR_setsgroups 287 diff --git a/memcheck/tests/darwin/scalar.stderr.exp b/memcheck/tests/darwin/scalar.stderr.exp index 348477bb85..001cc737b0 100644 --- a/memcheck/tests/darwin/scalar.stderr.exp +++ b/memcheck/tests/darwin/scalar.stderr.exp @@ -748,6 +748,25 @@ Syscall param fstat_extended(fsacl) points to unaddressable byte(s) ... Address 0x........ is not stack'd, malloc'd or (recently) free'd +----------------------------------------------------- +x200011c(284):__NR_access_extended 4s 1m +----------------------------------------------------- +Syscall param access_extended(entries) contains uninitialised byte(s) + ... + +Syscall param access_extended(size) contains uninitialised byte(s) + ... + +Syscall param access_extended(results) contains uninitialised byte(s) + ... + +Syscall param access_extended(uid) contains uninitialised byte(s) + ... + +Syscall param access_extended(entries) points to unaddressable byte(s) + ... + Address 0x........ is not stack'd, malloc'd or (recently) free'd + ----------------------------------------------------- (296): old load_shared_file ----------------------------------------------------- -- 2.47.2