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 <[email protected]>
---
fs/binfmt_misc.c | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
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;
--
2.25.1
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);
...
--
Kees Cook
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
--
Kees Cook