| Message ID | 20221102025123.1117184-1-liushixin2@huawei.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | binfmt_misc: fix shift-out-of-bounds in check_special_flags | expand |
On Wed, Nov 02, 2022 at 10:51:23AM +0800, Liu Shixin wrote: > UBSAN reported a shift-out-of-bounds warning: > > left shift of 1 by 31 places cannot be represented in type 'int' > Call Trace: > <TASK> > __dump_stack lib/dump_stack.c:88 [inline] > dump_stack_lvl+0x8d/0xcf lib/dump_stack.c:106 > ubsan_epilogue+0xa/0x44 lib/ubsan.c:151 > __ubsan_handle_shift_out_of_bounds+0x1e7/0x208 lib/ubsan.c:322 > check_special_flags fs/binfmt_misc.c:241 [inline] > create_entry fs/binfmt_misc.c:456 [inline] > bm_register_write+0x9d3/0xa20 fs/binfmt_misc.c:654 > vfs_write+0x11e/0x580 fs/read_write.c:582 > ksys_write+0xcf/0x120 fs/read_write.c:637 > do_syscall_x64 arch/x86/entry/common.c:50 [inline] > do_syscall_64+0x34/0x80 arch/x86/entry/common.c:80 > entry_SYSCALL_64_after_hwframe+0x63/0xcd > RIP: 0033:0x4194e1 > > Since the type of Node's flags is unsigned long, we should define these > macros with same type too. We are limited to 32 bits anyway. More interesting question here is what's the point of having those bits that high anyway?
On Sat, Nov 19, 2022 at 05:29:22AM +0000, Al Viro wrote: > On Wed, Nov 02, 2022 at 10:51:23AM +0800, Liu Shixin wrote: > > UBSAN reported a shift-out-of-bounds warning: > > > > left shift of 1 by 31 places cannot be represented in type 'int' > > Call Trace: > > <TASK> > > __dump_stack lib/dump_stack.c:88 [inline] > > dump_stack_lvl+0x8d/0xcf lib/dump_stack.c:106 > > ubsan_epilogue+0xa/0x44 lib/ubsan.c:151 > > __ubsan_handle_shift_out_of_bounds+0x1e7/0x208 lib/ubsan.c:322 > > check_special_flags fs/binfmt_misc.c:241 [inline] > > create_entry fs/binfmt_misc.c:456 [inline] > > bm_register_write+0x9d3/0xa20 fs/binfmt_misc.c:654 > > vfs_write+0x11e/0x580 fs/read_write.c:582 > > ksys_write+0xcf/0x120 fs/read_write.c:637 > > do_syscall_x64 arch/x86/entry/common.c:50 [inline] > > do_syscall_64+0x34/0x80 arch/x86/entry/common.c:80 > > entry_SYSCALL_64_after_hwframe+0x63/0xcd > > RIP: 0033:0x4194e1 > > > > Since the type of Node's flags is unsigned long, we should define these > > macros with same type too. > > We are limited to 32 bits anyway. More interesting question here is what's > the point of having those bits that high anyway? Hm, looks like it was designed to avoid the enum just before the defines: enum {Enabled, Magic}; #define MISC_FMT_PRESERVE_ARGV0 (1 << 31) #define MISC_FMT_OPEN_BINARY (1 << 30) #define MISC_FMT_CREDENTIALS (1 << 29) #define MISC_FMT_OPEN_FILE (1 << 28) But both appear to be entirely internally defined bits. I think these can all just be part of the enum, and we can quit mixing "&" tests and test_bit() calls: ... if (e->flags & MISC_FMT_OPEN_FILE) *dp++ = 'F'; *dp++ = '\n'; if (!test_bit(Magic, &e->flags)) { sprintf(dp, "extension .%s\n", e->magic); ...
On Wed, 2 Nov 2022 10:51:23 +0800, Liu Shixin wrote: > UBSAN reported a shift-out-of-bounds warning: > > left shift of 1 by 31 places cannot be represented in type 'int' > Call Trace: > <TASK> > __dump_stack lib/dump_stack.c:88 [inline] > dump_stack_lvl+0x8d/0xcf lib/dump_stack.c:106 > ubsan_epilogue+0xa/0x44 lib/ubsan.c:151 > __ubsan_handle_shift_out_of_bounds+0x1e7/0x208 lib/ubsan.c:322 > check_special_flags fs/binfmt_misc.c:241 [inline] > create_entry fs/binfmt_misc.c:456 [inline] > bm_register_write+0x9d3/0xa20 fs/binfmt_misc.c:654 > vfs_write+0x11e/0x580 fs/read_write.c:582 > ksys_write+0xcf/0x120 fs/read_write.c:637 > do_syscall_x64 arch/x86/entry/common.c:50 [inline] > do_syscall_64+0x34/0x80 arch/x86/entry/common.c:80 > entry_SYSCALL_64_after_hwframe+0x63/0xcd > RIP: 0033:0x4194e1 > > [...] Applied to for-next/execve, thanks! [1/1] binfmt_misc: fix shift-out-of-bounds in check_special_flags https://git.kernel.org/kees/c/6a46bf558803
diff --git a/fs/binfmt_misc.c b/fs/binfmt_misc.c index e1eae7ea823a..bb202ad369d5 100644 --- a/fs/binfmt_misc.c +++ b/fs/binfmt_misc.c @@ -44,10 +44,10 @@ static LIST_HEAD(entries); static int enabled = 1; enum {Enabled, Magic}; -#define MISC_FMT_PRESERVE_ARGV0 (1 << 31) -#define MISC_FMT_OPEN_BINARY (1 << 30) -#define MISC_FMT_CREDENTIALS (1 << 29) -#define MISC_FMT_OPEN_FILE (1 << 28) +#define MISC_FMT_PRESERVE_ARGV0 (1UL << 31) +#define MISC_FMT_OPEN_BINARY (1UL << 30) +#define MISC_FMT_CREDENTIALS (1UL << 29) +#define MISC_FMT_OPEN_FILE (1UL << 28) typedef struct { struct list_head list;
UBSAN reported a shift-out-of-bounds warning: left shift of 1 by 31 places cannot be represented in type 'int' Call Trace: <TASK> __dump_stack lib/dump_stack.c:88 [inline] dump_stack_lvl+0x8d/0xcf lib/dump_stack.c:106 ubsan_epilogue+0xa/0x44 lib/ubsan.c:151 __ubsan_handle_shift_out_of_bounds+0x1e7/0x208 lib/ubsan.c:322 check_special_flags fs/binfmt_misc.c:241 [inline] create_entry fs/binfmt_misc.c:456 [inline] bm_register_write+0x9d3/0xa20 fs/binfmt_misc.c:654 vfs_write+0x11e/0x580 fs/read_write.c:582 ksys_write+0xcf/0x120 fs/read_write.c:637 do_syscall_x64 arch/x86/entry/common.c:50 [inline] do_syscall_64+0x34/0x80 arch/x86/entry/common.c:80 entry_SYSCALL_64_after_hwframe+0x63/0xcd RIP: 0033:0x4194e1 Since the type of Node's flags is unsigned long, we should define these macros with same type too. Signed-off-by: Liu Shixin <liushixin2@huawei.com> --- fs/binfmt_misc.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-)