net/can/bcm.c | 16 ++++++++++------ 1 file changed, 10 insertions(+), 6 deletions(-)
Syzkaller reported the following issue:
=====================================================
BUG: KMSAN: uninit-value in aio_rw_done fs/aio.c:1520 [inline]
BUG: KMSAN: uninit-value in aio_write+0x899/0x950 fs/aio.c:1600
aio_rw_done fs/aio.c:1520 [inline]
aio_write+0x899/0x950 fs/aio.c:1600
io_submit_one+0x1d1c/0x3bf0 fs/aio.c:2019
__do_sys_io_submit fs/aio.c:2078 [inline]
__se_sys_io_submit+0x293/0x770 fs/aio.c:2048
__x64_sys_io_submit+0x92/0xd0 fs/aio.c:2048
do_syscall_x64 arch/x86/entry/common.c:50 [inline]
do_syscall_64+0x3d/0xb0 arch/x86/entry/common.c:80
entry_SYSCALL_64_after_hwframe+0x63/0xcd
Uninit was created at:
slab_post_alloc_hook mm/slab.h:766 [inline]
slab_alloc_node mm/slub.c:3452 [inline]
__kmem_cache_alloc_node+0x71f/0xce0 mm/slub.c:3491
__do_kmalloc_node mm/slab_common.c:967 [inline]
__kmalloc+0x11d/0x3b0 mm/slab_common.c:981
kmalloc_array include/linux/slab.h:636 [inline]
bcm_tx_setup+0x80e/0x29d0 net/can/bcm.c:930
bcm_sendmsg+0x3a2/0xce0 net/can/bcm.c:1351
sock_sendmsg_nosec net/socket.c:714 [inline]
sock_sendmsg net/socket.c:734 [inline]
sock_write_iter+0x495/0x5e0 net/socket.c:1108
call_write_iter include/linux/fs.h:2189 [inline]
aio_write+0x63a/0x950 fs/aio.c:1600
io_submit_one+0x1d1c/0x3bf0 fs/aio.c:2019
__do_sys_io_submit fs/aio.c:2078 [inline]
__se_sys_io_submit+0x293/0x770 fs/aio.c:2048
__x64_sys_io_submit+0x92/0xd0 fs/aio.c:2048
do_syscall_x64 arch/x86/entry/common.c:50 [inline]
do_syscall_64+0x3d/0xb0 arch/x86/entry/common.c:80
entry_SYSCALL_64_after_hwframe+0x63/0xcd
CPU: 1 PID: 5034 Comm: syz-executor350 Not tainted 6.2.0-rc6-syzkaller-80422-geda666ff2276 #0
Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 01/12/2023
=====================================================
We can follow the call chain and find that 'bcm_tx_setup' function calls 'memcpy_from_msg'
to copy some content to the newly allocated frame of 'op->frames'. After that the 'len'
field of copied structure being compared with some constant value (64 or 8). However, if
'memcpy_from_msg' returns an error, we will compare some uninitialized memory. This triggers
'uninit-value' issue.
This patch will add 'memcpy_from_msg' possible errors processing to avoid uninit-value
issue.
Tested via syzkaller
Reported-by: syzbot+c9bfd85eca611ebf5db1@syzkaller.appspotmail.com
Link: https://syzkaller.appspot.com/bug?id=47f897f8ad958bbde5790ebf389b5e7e0a345089
Signed-off-by: Ivan Orlov <ivan.orlov0322@gmail.com>
---
net/can/bcm.c | 16 ++++++++++------
1 file changed, 10 insertions(+), 6 deletions(-)
diff --git a/net/can/bcm.c b/net/can/bcm.c
index 27706f6ace34..a962ec2b8ba5 100644
--- a/net/can/bcm.c
+++ b/net/can/bcm.c
@@ -941,6 +941,8 @@ static int bcm_tx_setup(struct bcm_msg_head *msg_head, struct msghdr *msg,
cf = op->frames + op->cfsiz * i;
err = memcpy_from_msg((u8 *)cf, msg, op->cfsiz);
+ if (err < 0)
+ goto free_op;
if (op->flags & CAN_FD_FRAME) {
if (cf->len > 64)
@@ -950,12 +952,8 @@ static int bcm_tx_setup(struct bcm_msg_head *msg_head, struct msghdr *msg,
err = -EINVAL;
}
- if (err < 0) {
- if (op->frames != &op->sframe)
- kfree(op->frames);
- kfree(op);
- return err;
- }
+ if (err < 0)
+ goto free_op;
if (msg_head->flags & TX_CP_CAN_ID) {
/* copy can_id into frame */
@@ -1026,6 +1024,12 @@ static int bcm_tx_setup(struct bcm_msg_head *msg_head, struct msghdr *msg,
bcm_tx_start_timer(op);
return msg_head->nframes * op->cfsiz + MHSIZ;
+
+free_op:
+ if (op->frames != &op->sframe)
+ kfree(op->frames);
+ kfree(op);
+ return err;
}
/*
--
2.34.1On 14.03.2023 16:04:45, Ivan Orlov wrote: > Syzkaller reported the following issue: > > ===================================================== > BUG: KMSAN: uninit-value in aio_rw_done fs/aio.c:1520 [inline] > BUG: KMSAN: uninit-value in aio_write+0x899/0x950 fs/aio.c:1600 > aio_rw_done fs/aio.c:1520 [inline] > aio_write+0x899/0x950 fs/aio.c:1600 > io_submit_one+0x1d1c/0x3bf0 fs/aio.c:2019 > __do_sys_io_submit fs/aio.c:2078 [inline] > __se_sys_io_submit+0x293/0x770 fs/aio.c:2048 > __x64_sys_io_submit+0x92/0xd0 fs/aio.c:2048 > do_syscall_x64 arch/x86/entry/common.c:50 [inline] > do_syscall_64+0x3d/0xb0 arch/x86/entry/common.c:80 > entry_SYSCALL_64_after_hwframe+0x63/0xcd > > Uninit was created at: > slab_post_alloc_hook mm/slab.h:766 [inline] > slab_alloc_node mm/slub.c:3452 [inline] > __kmem_cache_alloc_node+0x71f/0xce0 mm/slub.c:3491 > __do_kmalloc_node mm/slab_common.c:967 [inline] > __kmalloc+0x11d/0x3b0 mm/slab_common.c:981 > kmalloc_array include/linux/slab.h:636 [inline] > bcm_tx_setup+0x80e/0x29d0 net/can/bcm.c:930 > bcm_sendmsg+0x3a2/0xce0 net/can/bcm.c:1351 > sock_sendmsg_nosec net/socket.c:714 [inline] > sock_sendmsg net/socket.c:734 [inline] > sock_write_iter+0x495/0x5e0 net/socket.c:1108 > call_write_iter include/linux/fs.h:2189 [inline] > aio_write+0x63a/0x950 fs/aio.c:1600 > io_submit_one+0x1d1c/0x3bf0 fs/aio.c:2019 > __do_sys_io_submit fs/aio.c:2078 [inline] > __se_sys_io_submit+0x293/0x770 fs/aio.c:2048 > __x64_sys_io_submit+0x92/0xd0 fs/aio.c:2048 > do_syscall_x64 arch/x86/entry/common.c:50 [inline] > do_syscall_64+0x3d/0xb0 arch/x86/entry/common.c:80 > entry_SYSCALL_64_after_hwframe+0x63/0xcd > > CPU: 1 PID: 5034 Comm: syz-executor350 Not tainted 6.2.0-rc6-syzkaller-80422-geda666ff2276 #0 > Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 01/12/2023 > ===================================================== > > We can follow the call chain and find that 'bcm_tx_setup' function calls 'memcpy_from_msg' > to copy some content to the newly allocated frame of 'op->frames'. After that the 'len' > field of copied structure being compared with some constant value (64 or 8). However, if > 'memcpy_from_msg' returns an error, we will compare some uninitialized memory. This triggers > 'uninit-value' issue. > > This patch will add 'memcpy_from_msg' possible errors processing to avoid uninit-value > issue. > > Tested via syzkaller > > Reported-by: syzbot+c9bfd85eca611ebf5db1@syzkaller.appspotmail.com > Link: https://syzkaller.appspot.com/bug?id=47f897f8ad958bbde5790ebf389b5e7e0a345089 > Signed-off-by: Ivan Orlov <ivan.orlov0322@gmail.com> Applied to linux-can. Thanks, Marc -- Pengutronix e.K. | Marc Kleine-Budde | Embedded Linux | https://www.pengutronix.de | Vertretung Nürnberg | Phone: +49-5121-206917-129 | Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 |
Hello Ivan,
besides the fact that we would read some uninitialized value the outcome
of the original implementation would have been an error and a
termination of the copy process too. Maybe throwing a different error
number.
But it is really interesting to see what KMSAN is able to detect these
days! Many thanks for the finding and your effort to contribute this fix!
Best regards,
Oliver
On 14.03.23 13:04, Ivan Orlov wrote:
> Syzkaller reported the following issue:
(..)
>
> Reported-by: syzbot+c9bfd85eca611ebf5db1@syzkaller.appspotmail.com
> Link: https://syzkaller.appspot.com/bug?id=47f897f8ad958bbde5790ebf389b5e7e0a345089
> Signed-off-by: Ivan Orlov <ivan.orlov0322@gmail.com>
Acked-by: Oliver Hartkopp <socketcan@hartkopp.net>
> ---
> net/can/bcm.c | 16 ++++++++++------
> 1 file changed, 10 insertions(+), 6 deletions(-)
>
> diff --git a/net/can/bcm.c b/net/can/bcm.c
> index 27706f6ace34..a962ec2b8ba5 100644
> --- a/net/can/bcm.c
> +++ b/net/can/bcm.c
> @@ -941,6 +941,8 @@ static int bcm_tx_setup(struct bcm_msg_head *msg_head, struct msghdr *msg,
>
> cf = op->frames + op->cfsiz * i;
> err = memcpy_from_msg((u8 *)cf, msg, op->cfsiz);
> + if (err < 0)
> + goto free_op;
>
> if (op->flags & CAN_FD_FRAME) {
> if (cf->len > 64)
> @@ -950,12 +952,8 @@ static int bcm_tx_setup(struct bcm_msg_head *msg_head, struct msghdr *msg,
> err = -EINVAL;
> }
>
> - if (err < 0) {
> - if (op->frames != &op->sframe)
> - kfree(op->frames);
> - kfree(op);
> - return err;
> - }
> + if (err < 0)
> + goto free_op;
>
> if (msg_head->flags & TX_CP_CAN_ID) {
> /* copy can_id into frame */
> @@ -1026,6 +1024,12 @@ static int bcm_tx_setup(struct bcm_msg_head *msg_head, struct msghdr *msg,
> bcm_tx_start_timer(op);
>
> return msg_head->nframes * op->cfsiz + MHSIZ;
> +
> +free_op:
> + if (op->frames != &op->sframe)
> + kfree(op->frames);
> + kfree(op);
> + return err;
> }
>
> /*
On 3/14/23 18:38, Oliver Hartkopp wrote:
> Hello Ivan,
>
> besides the fact that we would read some uninitialized value the outcome
> of the original implementation would have been an error and a
> termination of the copy process too. Maybe throwing a different error
> number.
>
> But it is really interesting to see what KMSAN is able to detect these
> days! Many thanks for the finding and your effort to contribute this fix!
>
> Best regards,
> Oliver
>
>
> On 14.03.23 13:04, Ivan Orlov wrote:
>> Syzkaller reported the following issue:
>
> (..)
>
>>
>> Reported-by: syzbot+c9bfd85eca611ebf5db1@syzkaller.appspotmail.com
>> Link:
>> https://syzkaller.appspot.com/bug?id=47f897f8ad958bbde5790ebf389b5e7e0a345089
>> Signed-off-by: Ivan Orlov <ivan.orlov0322@gmail.com>
>
> Acked-by: Oliver Hartkopp <socketcan@hartkopp.net>
>
>
>> ---
>> net/can/bcm.c | 16 ++++++++++------
>> 1 file changed, 10 insertions(+), 6 deletions(-)
>>
>> diff --git a/net/can/bcm.c b/net/can/bcm.c
>> index 27706f6ace34..a962ec2b8ba5 100644
>> --- a/net/can/bcm.c
>> +++ b/net/can/bcm.c
>> @@ -941,6 +941,8 @@ static int bcm_tx_setup(struct bcm_msg_head
>> *msg_head, struct msghdr *msg,
>> cf = op->frames + op->cfsiz * i;
>> err = memcpy_from_msg((u8 *)cf, msg, op->cfsiz);
>> + if (err < 0)
>> + goto free_op;
>> if (op->flags & CAN_FD_FRAME) {
>> if (cf->len > 64)
>> @@ -950,12 +952,8 @@ static int bcm_tx_setup(struct bcm_msg_head
>> *msg_head, struct msghdr *msg,
>> err = -EINVAL;
>> }
>> - if (err < 0) {
>> - if (op->frames != &op->sframe)
>> - kfree(op->frames);
>> - kfree(op);
>> - return err;
>> - }
>> + if (err < 0)
>> + goto free_op;
>> if (msg_head->flags & TX_CP_CAN_ID) {
>> /* copy can_id into frame */
>> @@ -1026,6 +1024,12 @@ static int bcm_tx_setup(struct bcm_msg_head
>> *msg_head, struct msghdr *msg,
>> bcm_tx_start_timer(op);
>> return msg_head->nframes * op->cfsiz + MHSIZ;
>> +
>> +free_op:
>> + if (op->frames != &op->sframe)
>> + kfree(op->frames);
>> + kfree(op);
>> + return err;
>> }
>> /*
Thank you for the quick answer! I totally agree that this patch will not
change the behavior a lot. However, I think a little bit more error
processing will not be bad (considering this will not bring any
performance overhead). If someone in the future tries to use the "cf"
object right after "memcpy_from_msg" call without proper error
processing it will lead to a bug (which will be hard to trigger). Maybe
fixing it now to avoid possible future mistakes in the future makes sense?
On 14.03.23 16:37, Ivan Orlov wrote:
> On 3/14/23 18:38, Oliver Hartkopp wrote:
>> Hello Ivan,
>>
>> besides the fact that we would read some uninitialized value the
>> outcome of the original implementation would have been an error and a
>> termination of the copy process too. Maybe throwing a different error
>> number.
>>
>> But it is really interesting to see what KMSAN is able to detect these
>> days! Many thanks for the finding and your effort to contribute this fix!
>>
>> Best regards,
>> Oliver
>>
>>
>> On 14.03.23 13:04, Ivan Orlov wrote:
>>> Syzkaller reported the following issue:
>>
>> (..)
>>
>>>
>>> Reported-by: syzbot+c9bfd85eca611ebf5db1@syzkaller.appspotmail.com
>>> Link:
>>> https://syzkaller.appspot.com/bug?id=47f897f8ad958bbde5790ebf389b5e7e0a345089
>>> Signed-off-by: Ivan Orlov <ivan.orlov0322@gmail.com>
>>
>> Acked-by: Oliver Hartkopp <socketcan@hartkopp.net>
>>
>>
>>> ---
>>> net/can/bcm.c | 16 ++++++++++------
>>> 1 file changed, 10 insertions(+), 6 deletions(-)
>>>
>>> diff --git a/net/can/bcm.c b/net/can/bcm.c
>>> index 27706f6ace34..a962ec2b8ba5 100644
>>> --- a/net/can/bcm.c
>>> +++ b/net/can/bcm.c
>>> @@ -941,6 +941,8 @@ static int bcm_tx_setup(struct bcm_msg_head
>>> *msg_head, struct msghdr *msg,
>>> cf = op->frames + op->cfsiz * i;
>>> err = memcpy_from_msg((u8 *)cf, msg, op->cfsiz);
>>> + if (err < 0)
>>> + goto free_op;
>>> if (op->flags & CAN_FD_FRAME) {
>>> if (cf->len > 64)
>>> @@ -950,12 +952,8 @@ static int bcm_tx_setup(struct bcm_msg_head
>>> *msg_head, struct msghdr *msg,
>>> err = -EINVAL;
>>> }
>>> - if (err < 0) {
>>> - if (op->frames != &op->sframe)
>>> - kfree(op->frames);
>>> - kfree(op);
>>> - return err;
>>> - }
>>> + if (err < 0)
>>> + goto free_op;
>>> if (msg_head->flags & TX_CP_CAN_ID) {
>>> /* copy can_id into frame */
>>> @@ -1026,6 +1024,12 @@ static int bcm_tx_setup(struct bcm_msg_head
>>> *msg_head, struct msghdr *msg,
>>> bcm_tx_start_timer(op);
>>> return msg_head->nframes * op->cfsiz + MHSIZ;
>>> +
>>> +free_op:
>>> + if (op->frames != &op->sframe)
>>> + kfree(op->frames);
>>> + kfree(op);
>>> + return err;
>>> }
>>> /*
>
> Thank you for the quick answer! I totally agree that this patch will not
> change the behavior a lot. However, I think a little bit more error
> processing will not be bad (considering this will not bring any
> performance overhead). If someone in the future tries to use the "cf"
> object right after "memcpy_from_msg" call without proper error
> processing it will lead to a bug (which will be hard to trigger). Maybe
> fixing it now to avoid possible future mistakes in the future makes sense?
Yes! Definitely!
Therefore I added my Acked-by: tag. Marc will likely pick this patch for
upstream.
Best regards,
Oliver
On 14.03.2023 20:23:47, Oliver Hartkopp wrote:
>
>
> On 14.03.23 16:37, Ivan Orlov wrote:
> > On 3/14/23 18:38, Oliver Hartkopp wrote:
> > > Hello Ivan,
> > >
> > > besides the fact that we would read some uninitialized value the
> > > outcome of the original implementation would have been an error and
> > > a termination of the copy process too. Maybe throwing a different
> > > error number.
> > >
> > > But it is really interesting to see what KMSAN is able to detect
> > > these days! Many thanks for the finding and your effort to
> > > contribute this fix!
> > >
> > > Best regards,
> > > Oliver
> > >
> > >
> > > On 14.03.23 13:04, Ivan Orlov wrote:
> > > > Syzkaller reported the following issue:
> > >
> > > (..)
> > >
> > > >
> > > > Reported-by: syzbot+c9bfd85eca611ebf5db1@syzkaller.appspotmail.com
> > > > Link: https://syzkaller.appspot.com/bug?id=47f897f8ad958bbde5790ebf389b5e7e0a345089
> > > > Signed-off-by: Ivan Orlov <ivan.orlov0322@gmail.com>
> > >
> > > Acked-by: Oliver Hartkopp <socketcan@hartkopp.net>
> > >
> > >
> > > > ---
> > > > net/can/bcm.c | 16 ++++++++++------
> > > > 1 file changed, 10 insertions(+), 6 deletions(-)
> > > >
> > > > diff --git a/net/can/bcm.c b/net/can/bcm.c
> > > > index 27706f6ace34..a962ec2b8ba5 100644
> > > > --- a/net/can/bcm.c
> > > > +++ b/net/can/bcm.c
> > > > @@ -941,6 +941,8 @@ static int bcm_tx_setup(struct bcm_msg_head
> > > > *msg_head, struct msghdr *msg,
> > > > cf = op->frames + op->cfsiz * i;
> > > > err = memcpy_from_msg((u8 *)cf, msg, op->cfsiz);
> > > > + if (err < 0)
> > > > + goto free_op;
> > > > if (op->flags & CAN_FD_FRAME) {
> > > > if (cf->len > 64)
> > > > @@ -950,12 +952,8 @@ static int bcm_tx_setup(struct bcm_msg_head
> > > > *msg_head, struct msghdr *msg,
> > > > err = -EINVAL;
> > > > }
> > > > - if (err < 0) {
> > > > - if (op->frames != &op->sframe)
> > > > - kfree(op->frames);
> > > > - kfree(op);
> > > > - return err;
> > > > - }
> > > > + if (err < 0)
> > > > + goto free_op;
> > > > if (msg_head->flags & TX_CP_CAN_ID) {
> > > > /* copy can_id into frame */
> > > > @@ -1026,6 +1024,12 @@ static int bcm_tx_setup(struct
> > > > bcm_msg_head *msg_head, struct msghdr *msg,
> > > > bcm_tx_start_timer(op);
> > > > return msg_head->nframes * op->cfsiz + MHSIZ;
> > > > +
> > > > +free_op:
> > > > + if (op->frames != &op->sframe)
> > > > + kfree(op->frames);
> > > > + kfree(op);
> > > > + return err;
> > > > }
> > > > /*
> >
> > Thank you for the quick answer! I totally agree that this patch will not
> > change the behavior a lot. However, I think a little bit more error
> > processing will not be bad (considering this will not bring any
> > performance overhead). If someone in the future tries to use the "cf"
> > object right after "memcpy_from_msg" call without proper error
> > processing it will lead to a bug (which will be hard to trigger). Maybe
> > fixing it now to avoid possible future mistakes in the future makes
> > sense?
>
> Yes! Definitely!
>
> Therefore I added my Acked-by: tag. Marc will likely pick this patch for
> upstream.
Can you create a proper Fixes tag?
Marc
--
Pengutronix e.K. | Marc Kleine-Budde |
Embedded Linux | https://www.pengutronix.de |
Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 |
On 20.03.23 16:40, Marc Kleine-Budde wrote:
> On 14.03.2023 20:23:47, Oliver Hartkopp wrote:
>>
>>
>> On 14.03.23 16:37, Ivan Orlov wrote:
>>> On 3/14/23 18:38, Oliver Hartkopp wrote:
>>>> Hello Ivan,
>>>>
>>>> besides the fact that we would read some uninitialized value the
>>>> outcome of the original implementation would have been an error and
>>>> a termination of the copy process too. Maybe throwing a different
>>>> error number.
>>>>
>>>> But it is really interesting to see what KMSAN is able to detect
>>>> these days! Many thanks for the finding and your effort to
>>>> contribute this fix!
>>>>
>>>> Best regards,
>>>> Oliver
>>>>
>>>>
>>>> On 14.03.23 13:04, Ivan Orlov wrote:
>>>>> Syzkaller reported the following issue:
>>>>
>>>> (..)
>>>>
>>>>>
>>>>> Reported-by: syzbot+c9bfd85eca611ebf5db1@syzkaller.appspotmail.com
>>>>> Link: https://syzkaller.appspot.com/bug?id=47f897f8ad958bbde5790ebf389b5e7e0a345089
>>>>> Signed-off-by: Ivan Orlov <ivan.orlov0322@gmail.com>
>>>>
>>>> Acked-by: Oliver Hartkopp <socketcan@hartkopp.net>
>>>>
>>>>
>>>>> ---
>>>>> net/can/bcm.c | 16 ++++++++++------
>>>>> 1 file changed, 10 insertions(+), 6 deletions(-)
>>>>>
>>>>> diff --git a/net/can/bcm.c b/net/can/bcm.c
>>>>> index 27706f6ace34..a962ec2b8ba5 100644
>>>>> --- a/net/can/bcm.c
>>>>> +++ b/net/can/bcm.c
>>>>> @@ -941,6 +941,8 @@ static int bcm_tx_setup(struct bcm_msg_head
>>>>> *msg_head, struct msghdr *msg,
>>>>> cf = op->frames + op->cfsiz * i;
>>>>> err = memcpy_from_msg((u8 *)cf, msg, op->cfsiz);
>>>>> + if (err < 0)
>>>>> + goto free_op;
>>>>> if (op->flags & CAN_FD_FRAME) {
>>>>> if (cf->len > 64)
>>>>> @@ -950,12 +952,8 @@ static int bcm_tx_setup(struct bcm_msg_head
>>>>> *msg_head, struct msghdr *msg,
>>>>> err = -EINVAL;
>>>>> }
>>>>> - if (err < 0) {
>>>>> - if (op->frames != &op->sframe)
>>>>> - kfree(op->frames);
>>>>> - kfree(op);
>>>>> - return err;
>>>>> - }
>>>>> + if (err < 0)
>>>>> + goto free_op;
>>>>> if (msg_head->flags & TX_CP_CAN_ID) {
>>>>> /* copy can_id into frame */
>>>>> @@ -1026,6 +1024,12 @@ static int bcm_tx_setup(struct
>>>>> bcm_msg_head *msg_head, struct msghdr *msg,
>>>>> bcm_tx_start_timer(op);
>>>>> return msg_head->nframes * op->cfsiz + MHSIZ;
>>>>> +
>>>>> +free_op:
>>>>> + if (op->frames != &op->sframe)
>>>>> + kfree(op->frames);
>>>>> + kfree(op);
>>>>> + return err;
>>>>> }
>>>>> /*
>>>
>>> Thank you for the quick answer! I totally agree that this patch will not
>>> change the behavior a lot. However, I think a little bit more error
>>> processing will not be bad (considering this will not bring any
>>> performance overhead). If someone in the future tries to use the "cf"
>>> object right after "memcpy_from_msg" call without proper error
>>> processing it will lead to a bug (which will be hard to trigger). Maybe
>>> fixing it now to avoid possible future mistakes in the future makes
>>> sense?
>>
>> Yes! Definitely!
>>
>> Therefore I added my Acked-by: tag. Marc will likely pick this patch for
>> upstream.
>
> Can you create a proper Fixes tag?
Fixes: 6f3b911d5f29b ("can: bcm: add support for CAN FD frames")
Btw. do you really think this is a candidate for stable?
IMHO it is just a KMSAN hit that should be fixed for future releases ...
Best regards,
Oliver
© 2016 - 2026 Red Hat, Inc.