Re: [PATCH net-next V2] can: j1939: fix uaf warning in j1939_session_destroy

[Sabyrzhan Tasbolatov]

On Thu, 8 Aug 2024 19:07:55 +0800, Edward Adam Davis wrote:
> On Thu, 8 Aug 2024 09:49:18 +0200, Oleksij Rempel wrote:
> > > the skb to the queue and increase the skb reference count through it.
> > > 
> > > Reported-and-tested-by: syzbot+ad601904231505ad6617@xxxxxxxxxxxxxxxxxxxxxxxxx
> > > Closes: https://syzkaller.appspot.com/bug?extid=ad601904231505ad6617
> > > Signed-off-by: Edward Adam Davis <eadavis@xxxxxx>
> > 
> > This patch breaks j1939.
> > The issue can be reproduced by running following commands:
> I tried to reproduce the problem using the following command, but was 
> unsuccessful. Prompt me to install j1939cat and j1939acd, and there are
> some other errors.
> 
> Can you share the logs from when you reproduced the problem?

Hello,

Here is the log of can-tests/j1939/run_all.sh:

# ip link add type vcan
# ip l s dev vcan0 up
# ./run_all.sh vcan0 vcan0
##############################################
run: j1939_ac_100k_dual_can.sh
generate random data for the test
1+0 records in
1+0 records out
102400 bytes (102 kB, 100 KiB) copied, 0.00191192 s, 53.6 MB/s
start j1939acd and j1939cat on vcan0
8321
8323
start j1939acd and j1939cat on vcan0
[  132.211317][ T8326] vcan0: tx drop: invalid sa for name 0x0000000011223340
j1939cat: j1939cat_send_one: transfer error: -99: Cannot assign requested address

It fails here:
https://github.com/linux-can/can-tests/blob/master/j1939/j1939_ac_100k_dual_can.sh#L70

The error message is printed in this condition:
https://elixir.bootlin.com/linux/v6.12-rc2/source/net/can/j1939/address-claim.c#L104-L108

I've applied your patch on the current 6.12.0-rc2 and the syzkaller C repro
doesn't trigger WARNING uaf, refcount anymore though.

== Offtopic:
I wonder if can-tests/j1939 should be refactored from shell to C tests in the
same linux-can/can-tests repository (or even migrate to KUnit tests)
to improve debugging, test coverage. I'd like to understand which syscalls
and params are used j1939cat and j1939acd utils -- currently, tracing with
strace and trace-cmd (ftrace).

> > git clone git@xxxxxxxxxx:linux-can/can-tests.git
> > cd can-tests/j1939/
> > ip link add type vcan
> > ip l s dev vcan0 up
> > ./run_all.sh vcan0 vcan0