On Mon, Aug 06, 2018 at 11:45:29AM -0700, Linus Torvalds wrote: > On Mon, Aug 6, 2018 at 10:06 AM Al Viro <viro@xxxxxxxxxxxxxxxxxx> wrote: > > > > That leaves us with f_bsize and f_frsize (the latter defaulting to the former). > > Hugetlbfs can put greater than 4Gb values in there, for really huge pages. > > And that's the only thing worth checking in there. > > > > So the whole put_compat_statfs64() thing should be > > Ack, I'm ok with this simplification. > > > I'm somewhat tempted to get rid of those 'long' in struct kstatfs, > > I'm ok with this one too. > > > with > > > > #define STATFS_COPYOUT(type) \ > > static int put##type(struct kstatfs *st, struct type __user *p) \ > > No. Don't do this. > > I'm ok with the #define to avoid duplication, but don't bother with > the FIT_IN() after you've above successfully argued that it's > pointless for anything but f_bsize/frsize. > > So if you do the macro to generate the different copyout versions, > just use your simplified code for that macro instead. With FIT_IN() > just for f_bsize/frsize. For statfs64 (both native and compat) that would do nicely; for statfs, however... The following describes the field sizes in all that mess: kstatfs statfs statfs64 compat_statfs compat_statfs64 !s390 s390 !s390 s390 type: W W 32 W 32 32 32 namelen:W W 32 W 32 32 32 flags: W W 32 W 32 32 32 bsize: W W 32 W 32 32 32 frsize: W W 32 W 32 32 32 blocks: 64 W 64 64 64 32 64 bfree: 64 W 64 64 64 32 64 bavail: 64 W 64 64 64 32 64 files: 64 W 64 64 64 32 64 ffree: 64 W 64 64 64 32 64 fsid: __kernel_fsid_t on all First of all, nobody gives a fuck about type, namelen and flags overflows - can't happen. blocks/bfree/bavail/files/ffree: can overflow in plain statfs on 32bit and in compat statfs. For files/ffree we also have "-1 means (s32)-1, not an overflow" there. bsize/frsize: can oveflow in anything on s390 or mips64 and any compat on anything Oh, and then there's signedness - in compat_statfs64 everything's unsigned, but for compat_statfs a bunch of those 32bit ones are signed. Native on 32bit tend to be unsigned; native on 64bit and compat are often signed. IMO that's a bug - compat ones should all be same as native 32bit. As it is, arm, parisc, ppc, sparc, x86: on 32bit - unsigned, compat on 64bit - signed mips: both signed s390: both unsigned I think we want to switch compat_statfs fields on the first group to u32. These declarations are not exposed to userland anyway. mips is interesting - I've no idea how does mips32 userland react to e.g. statfs() on 3G block filesystem...