Re: [PATCH] can: raw: add locking for raw flags bitfield

[Oliver Hartkopp <socketcan@hartkopp.net>]

Hello Vincent!

On 06.05.26 12:22, Vincent Mailhol wrote:
> On Mon. 4 May 2026 at 13:31, Oliver Hartkopp <socketcan@hartkopp.net> wrote:
>> With commit 890e5198a6e5 ("can: raw: use bitfields to store flags in
>> struct raw_sock") the formerly separate integer values have been integrated
>> into a single bitfield. This led to a read-modify-write operation when
>> changing a flag in raw_setsockopt() which now needs a locking to prevent
>> concurrent access.
>>
>> Instead of adding a lock/unlock hell in each of the flag manipulations this
>> patch introduces a wrapper for a new raw_setsockopt_locked() function
>> analogue to the isotp_setsockopt[_locked]() approach in net/can/isotp.c
>>
>> Fixes: 890e5198a6e5 ("can: raw: use bitfields to store flags in struct raw_sock")
> 
> Arg, that's my patch, sorry for that!

No problem. I did realize this either o_O

>> Reported-by: Eulgyu Kim <eulgyukim@snu.ac.kr>
> 
> Maybe add a link to the report?
> 
> Link: https://lore.kernel.org/linux-can/20260503112200.22727-1-eulgyukim@snu.ac.kr/

Good idea. In fact checkpatch.pl said "Reported-by: should be 
immediately followed by Closes: with a URL to the report"

So it should be

Closes: 
https://lore.kernel.org/linux-can/20260503112200.22727-1-eulgyukim@snu.ac.kr/

>> Tested-by: Eulgyu Kim <eulgyukim@snu.ac.kr>
>> Signed-off-by: Oliver Hartkopp <socketcan@hartkopp.net>
> 
> I was able to trigger the bug locally using Eulgyu minimum reproducer
> and I can confirm that the issue is correctly resolved by this patch.
> I also think that globally holding the lock simplifies the logic. I
> tried to think if there were any alternatives (like
> READ_ONCE()/WRITE_ONCE() or atomic types) but none of these seem
> applicable here.
> 
> Reviewed-by: Vincent Mailhol <mailhol@kernel.org>
> Tested-by: Vincent Mailhol <mailhol@kernel.org>

Thanks Vincent!

Best regards,
Oliver