| Message ID | 20221031112536.4177761-1-cuigaosheng1@huawei.com (mailing list archive) |
|---|---|
| State | Accepted |
| Delegated to: | Paul Moore |
| Headers | show |
| Series | capabilities: fix undefined behavior in bit shift for CAP_TO_MASK | expand |
Acked-by: Andrew G. Morgan <morgan@kernel.org> On Mon, Oct 31, 2022 at 4:25 AM Gaosheng Cui <cuigaosheng1@huawei.com> wrote: > > Shifting signed 32-bit value by 31 bits is undefined, so changing > significant bit to unsigned. The UBSAN warning calltrace like below: > > UBSAN: shift-out-of-bounds in security/commoncap.c:1252:2 > left shift of 1 by 31 places cannot be represented in type 'int' > Call Trace: > <TASK> > dump_stack_lvl+0x7d/0xa5 > dump_stack+0x15/0x1b > ubsan_epilogue+0xe/0x4e > __ubsan_handle_shift_out_of_bounds+0x1e7/0x20c > cap_task_prctl+0x561/0x6f0 > security_task_prctl+0x5a/0xb0 > __x64_sys_prctl+0x61/0x8f0 > do_syscall_64+0x58/0x80 > entry_SYSCALL_64_after_hwframe+0x63/0xcd > </TASK> > > Fixes: e338d263a76a ("Add 64-bit capability support to the kernel") > Signed-off-by: Gaosheng Cui <cuigaosheng1@huawei.com> > --- > include/uapi/linux/capability.h | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/include/uapi/linux/capability.h b/include/uapi/linux/capability.h > index 463d1ba2232a..3d61a0ae055d 100644 > --- a/include/uapi/linux/capability.h > +++ b/include/uapi/linux/capability.h > @@ -426,7 +426,7 @@ struct vfs_ns_cap_data { > */ > > #define CAP_TO_INDEX(x) ((x) >> 5) /* 1 << 5 == bits in __u32 */ > -#define CAP_TO_MASK(x) (1 << ((x) & 31)) /* mask for indexed __u32 */ > +#define CAP_TO_MASK(x) (1U << ((x) & 31)) /* mask for indexed __u32 */ > > > #endif /* _UAPI_LINUX_CAPABILITY_H */ > -- > 2.25.1 >
On Mon, Oct 31, 2022 at 07:18:54AM -0700, Andrew G. Morgan wrote: > Acked-by: Andrew G. Morgan <morgan@kernel.org> > > > On Mon, Oct 31, 2022 at 4:25 AM Gaosheng Cui <cuigaosheng1@huawei.com> wrote: > > > > Shifting signed 32-bit value by 31 bits is undefined, so changing > > significant bit to unsigned. The UBSAN warning calltrace like below: > > > > UBSAN: shift-out-of-bounds in security/commoncap.c:1252:2 > > left shift of 1 by 31 places cannot be represented in type 'int' > > Call Trace: > > <TASK> > > dump_stack_lvl+0x7d/0xa5 > > dump_stack+0x15/0x1b > > ubsan_epilogue+0xe/0x4e > > __ubsan_handle_shift_out_of_bounds+0x1e7/0x20c > > cap_task_prctl+0x561/0x6f0 > > security_task_prctl+0x5a/0xb0 > > __x64_sys_prctl+0x61/0x8f0 > > do_syscall_64+0x58/0x80 > > entry_SYSCALL_64_after_hwframe+0x63/0xcd > > </TASK> > > > > Fixes: e338d263a76a ("Add 64-bit capability support to the kernel") > > Signed-off-by: Gaosheng Cui <cuigaosheng1@huawei.com> Reviewed-by: Serge Hallyn <serge@hallyn.com> Paul, do you mind including this in your lsm tree? thanks, -serge > > --- > > include/uapi/linux/capability.h | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > diff --git a/include/uapi/linux/capability.h b/include/uapi/linux/capability.h > > index 463d1ba2232a..3d61a0ae055d 100644 > > --- a/include/uapi/linux/capability.h > > +++ b/include/uapi/linux/capability.h > > @@ -426,7 +426,7 @@ struct vfs_ns_cap_data { > > */ > > > > #define CAP_TO_INDEX(x) ((x) >> 5) /* 1 << 5 == bits in __u32 */ > > -#define CAP_TO_MASK(x) (1 << ((x) & 31)) /* mask for indexed __u32 */ > > +#define CAP_TO_MASK(x) (1U << ((x) & 31)) /* mask for indexed __u32 */ > > > > > > #endif /* _UAPI_LINUX_CAPABILITY_H */ > > -- > > 2.25.1 > >
On Tue, Nov 1, 2022 at 12:30 AM Serge E. Hallyn <serge@hallyn.com> wrote: > On Mon, Oct 31, 2022 at 07:18:54AM -0700, Andrew G. Morgan wrote: > > Acked-by: Andrew G. Morgan <morgan@kernel.org> > > > > > > On Mon, Oct 31, 2022 at 4:25 AM Gaosheng Cui <cuigaosheng1@huawei.com> wrote: > > > > > > Shifting signed 32-bit value by 31 bits is undefined, so changing > > > significant bit to unsigned. The UBSAN warning calltrace like below: > > > > > > UBSAN: shift-out-of-bounds in security/commoncap.c:1252:2 > > > left shift of 1 by 31 places cannot be represented in type 'int' > > > Call Trace: > > > <TASK> > > > dump_stack_lvl+0x7d/0xa5 > > > dump_stack+0x15/0x1b > > > ubsan_epilogue+0xe/0x4e > > > __ubsan_handle_shift_out_of_bounds+0x1e7/0x20c > > > cap_task_prctl+0x561/0x6f0 > > > security_task_prctl+0x5a/0xb0 > > > __x64_sys_prctl+0x61/0x8f0 > > > do_syscall_64+0x58/0x80 > > > entry_SYSCALL_64_after_hwframe+0x63/0xcd > > > </TASK> > > > > > > Fixes: e338d263a76a ("Add 64-bit capability support to the kernel") > > > Signed-off-by: Gaosheng Cui <cuigaosheng1@huawei.com> > > Reviewed-by: Serge Hallyn <serge@hallyn.com> > > Paul, do you mind including this in your lsm tree? Sure, although just a warning that it might not happen until next week. Maybe I'll get some time this weekend but I can't be certain. > > > --- > > > include/uapi/linux/capability.h | 2 +- > > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > > > diff --git a/include/uapi/linux/capability.h b/include/uapi/linux/capability.h > > > index 463d1ba2232a..3d61a0ae055d 100644 > > > --- a/include/uapi/linux/capability.h > > > +++ b/include/uapi/linux/capability.h > > > @@ -426,7 +426,7 @@ struct vfs_ns_cap_data { > > > */ > > > > > > #define CAP_TO_INDEX(x) ((x) >> 5) /* 1 << 5 == bits in __u32 */ > > > -#define CAP_TO_MASK(x) (1 << ((x) & 31)) /* mask for indexed __u32 */ > > > +#define CAP_TO_MASK(x) (1U << ((x) & 31)) /* mask for indexed __u32 */ > > > > > > > > > #endif /* _UAPI_LINUX_CAPABILITY_H */ > > > -- > > > 2.25.1
On Thu, Nov 03, 2022 at 07:57:45AM -0400, Paul Moore wrote: > On Tue, Nov 1, 2022 at 12:30 AM Serge E. Hallyn <serge@hallyn.com> wrote: > > On Mon, Oct 31, 2022 at 07:18:54AM -0700, Andrew G. Morgan wrote: > > > Acked-by: Andrew G. Morgan <morgan@kernel.org> > > > > > > > > > On Mon, Oct 31, 2022 at 4:25 AM Gaosheng Cui <cuigaosheng1@huawei.com> wrote: > > > > > > > > Shifting signed 32-bit value by 31 bits is undefined, so changing > > > > significant bit to unsigned. The UBSAN warning calltrace like below: > > > > > > > > UBSAN: shift-out-of-bounds in security/commoncap.c:1252:2 > > > > left shift of 1 by 31 places cannot be represented in type 'int' > > > > Call Trace: > > > > <TASK> > > > > dump_stack_lvl+0x7d/0xa5 > > > > dump_stack+0x15/0x1b > > > > ubsan_epilogue+0xe/0x4e > > > > __ubsan_handle_shift_out_of_bounds+0x1e7/0x20c > > > > cap_task_prctl+0x561/0x6f0 > > > > security_task_prctl+0x5a/0xb0 > > > > __x64_sys_prctl+0x61/0x8f0 > > > > do_syscall_64+0x58/0x80 > > > > entry_SYSCALL_64_after_hwframe+0x63/0xcd > > > > </TASK> > > > > > > > > Fixes: e338d263a76a ("Add 64-bit capability support to the kernel") > > > > Signed-off-by: Gaosheng Cui <cuigaosheng1@huawei.com> > > > > Reviewed-by: Serge Hallyn <serge@hallyn.com> > > > > Paul, do you mind including this in your lsm tree? > > Sure, although just a warning that it might not happen until next > week. Maybe I'll get some time this weekend but I can't be certain. Ok, thanks. I wouldn't mind putting up a capability tree if number of patches were going to start ramping up, but historically that has not been worth it.
On Mon, Oct 31, 2022 at 7:25 AM Gaosheng Cui <cuigaosheng1@huawei.com> wrote: > > Shifting signed 32-bit value by 31 bits is undefined, so changing > significant bit to unsigned. The UBSAN warning calltrace like below: > > UBSAN: shift-out-of-bounds in security/commoncap.c:1252:2 > left shift of 1 by 31 places cannot be represented in type 'int' > Call Trace: > <TASK> > dump_stack_lvl+0x7d/0xa5 > dump_stack+0x15/0x1b > ubsan_epilogue+0xe/0x4e > __ubsan_handle_shift_out_of_bounds+0x1e7/0x20c > cap_task_prctl+0x561/0x6f0 > security_task_prctl+0x5a/0xb0 > __x64_sys_prctl+0x61/0x8f0 > do_syscall_64+0x58/0x80 > entry_SYSCALL_64_after_hwframe+0x63/0xcd > </TASK> > > Fixes: e338d263a76a ("Add 64-bit capability support to the kernel") > Signed-off-by: Gaosheng Cui <cuigaosheng1@huawei.com> > --- > include/uapi/linux/capability.h | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) Merged into lsm/stable-6.1, and I'll plan to send this up to Linus next week. Thanks everyone!
On Thu, Nov 3, 2022 at 9:24 AM Serge E. Hallyn <serge@hallyn.com> wrote: > Ok, thanks. > > I wouldn't mind putting up a capability tree if number of patches were > going to start ramping up, but historically that has not been worth it. Don't ever let me stop you from setting up a capability tree, but yeah, I understand why it probably wouldn't be very useful. As long as you keep doing the reviews, ACKs, etc. I don't mind carrying the capability patches in the LSM tree; merging stuff is usually the easy part of the job.
diff --git a/include/uapi/linux/capability.h b/include/uapi/linux/capability.h index 463d1ba2232a..3d61a0ae055d 100644 --- a/include/uapi/linux/capability.h +++ b/include/uapi/linux/capability.h @@ -426,7 +426,7 @@ struct vfs_ns_cap_data { */ #define CAP_TO_INDEX(x) ((x) >> 5) /* 1 << 5 == bits in __u32 */ -#define CAP_TO_MASK(x) (1 << ((x) & 31)) /* mask for indexed __u32 */ +#define CAP_TO_MASK(x) (1U << ((x) & 31)) /* mask for indexed __u32 */ #endif /* _UAPI_LINUX_CAPABILITY_H */
Shifting signed 32-bit value by 31 bits is undefined, so changing significant bit to unsigned. The UBSAN warning calltrace like below: UBSAN: shift-out-of-bounds in security/commoncap.c:1252:2 left shift of 1 by 31 places cannot be represented in type 'int' Call Trace: <TASK> dump_stack_lvl+0x7d/0xa5 dump_stack+0x15/0x1b ubsan_epilogue+0xe/0x4e __ubsan_handle_shift_out_of_bounds+0x1e7/0x20c cap_task_prctl+0x561/0x6f0 security_task_prctl+0x5a/0xb0 __x64_sys_prctl+0x61/0x8f0 do_syscall_64+0x58/0x80 entry_SYSCALL_64_after_hwframe+0x63/0xcd </TASK> Fixes: e338d263a76a ("Add 64-bit capability support to the kernel") Signed-off-by: Gaosheng Cui <cuigaosheng1@huawei.com> --- include/uapi/linux/capability.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)