Re: [PATCH] audit: fix undefined behavior in bit shift for AUDIT_BIT

> Thanks, this patch looks good, although the 'Fixes:' line above isn't
> correct.  The commit you reference may be the commit which created
> AUDIT_BIT in include/uapi/linux/audit.h, but before that it was
> defined in include/linux/audit.h with the original implementation
> shipping in v2.6.6-rc1.  As the original AUDIT_BIT implementation
> predates git itself, I would suggest removing the 'Fixes' line
> entirely and I'll simply add the usual stable marking when I merge it
> into audit/stable-6.1.  If that is okay with you I'll go ahead and
> merge this patch which that tweak - is that okay?

Yes, you are right, thanks for taking time to review this patch!

On 2022/10/31 19:34, Paul Moore wrote:
> On Sun, Oct 30, 2022 at 10:10 PM Gaosheng Cui <[email protected]> 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 kernel/auditfilter.c:179:23
>> 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
>>   audit_register_class+0x9d/0x137
>>   audit_classes_init+0x4d/0xb8
>>   do_one_initcall+0x76/0x430
>>   kernel_init_freeable+0x3b3/0x422
>>   kernel_init+0x24/0x1e0
>>   ret_from_fork+0x1f/0x30
>>   </TASK>
>>
>> Fixes: 607ca46e97a1 ("UAPI: (Scripted) Disintegrate include/linux")
> Thanks, this patch looks good, although the 'Fixes:' line above isn't
> correct.  The commit you reference may be the commit which created
> AUDIT_BIT in include/uapi/linux/audit.h, but before that it was
> defined in include/linux/audit.h with the original implementation
> shipping in v2.6.6-rc1.  As the original AUDIT_BIT implementation
> predates git itself, I would suggest removing the 'Fixes' line
> entirely and I'll simply add the usual stable marking when I merge it
> into audit/stable-6.1.  If that is okay with you I'll go ahead and
> merge this patch which that tweak - is that okay?
>
>> Signed-off-by: Gaosheng Cui <[email protected]>
>> ---
>>   include/uapi/linux/audit.h | 2 +-
>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/include/uapi/linux/audit.h b/include/uapi/linux/audit.h
>> index 7c1dc818b1d5..d676ed2b246e 100644
>> --- a/include/uapi/linux/audit.h
>> +++ b/include/uapi/linux/audit.h
>> @@ -187,7 +187,7 @@
>>   #define AUDIT_MAX_KEY_LEN  256
>>   #define AUDIT_BITMASK_SIZE 64
>>   #define AUDIT_WORD(nr) ((__u32)((nr)/32))
>> -#define AUDIT_BIT(nr)  (1 << ((nr) - AUDIT_WORD(nr)*32))
>> +#define AUDIT_BIT(nr)  (1U << ((nr) - AUDIT_WORD(nr)*32))
>>
>>   #define AUDIT_SYSCALL_CLASSES 16
>>   #define AUDIT_CLASS_DIR_WRITE 0
>> --
>> 2.25.1
> --
> paul-moore.com
>
> .

--
Linux-audit mailing list
[email protected]
https://listman.redhat.com/mailman/listinfo/linux-audit