Messages in this thread | | | Date | Thu, 04 Jan 2007 15:50:25 -0600 | From | Eric Sandeen <> | Subject | Re: [UPDATED PATCH] fix memory corruption from misinterpreted bad_inode_ops return values |
| |
Linus Torvalds wrote: > > On Thu, 4 Jan 2007, Eric Sandeen wrote: >> Andrew Morton wrote: >> >>> btw, couldn't we fix this bug with a simple old >>> >>> --- a/fs/bad_inode.c~a >>> +++ a/fs/bad_inode.c >>> @@ -15,7 +15,7 @@ >>> #include <linux/smp_lock.h> >>> #include <linux/namei.h> >>> >>> -static int return_EIO(void) >>> +static long return_EIO(void) >>> { >>> return -EIO; >>> } >>> _ >>> >>> ? >> What about ops that return loff_t (64 bits) on 32-bit arches and stuff >> it into 2 registers.... > > Do we actually have cases where we cast to a different return value?
Today, via the void * function casts in the bad file/inode ops, in effect yes.
static int return_EIO(void) { return -EIO; }
#define EIO_ERROR ((void *) (return_EIO))
... .listxattr = EIO_ERROR,
but listxattr is supposed to return a ssize_t, which is 64 bits on some platforms, and only 32 bits get filled in thanks to the (void *) cast. So we wind up with something other than the return value we expect...
Andrew's long suggestion breaks things the other way, with 64-bit returning ops on 32-bit arches which again only pick up the first 32 bits thanks to the (void *) cast.
If we're really happy with casting away the function arguments (which are not -used- in the bad_foo ops anyway), then I'd maybe suggest going back to my first try at this thing:
static int return_EIO_int(void) { return -EIO; } #define EIO_ERROR_INT ((void *) (return_EIO_int))
static struct inode_operations bad_inode_ops = { .create = EIO_ERROR_INT, ...etc...
which is most like what we have today, except with specific return types.
-Eric - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
| |