linux-kernel - Re: [PATCH] j1939: fix unable to send messages with data length zero

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <8412e625-6033-4ea2-84f1-209c82ae5866@gmx.net>
Date: Tue, 7 Jan 2025 18:17:13 +0100
From: Alexander Hölzl <alexander.hoelzl@....net>
To: Marc Kleine-Budde <mkl@...gutronix.de>
Cc: robin@...tonic.nl, socketcan@...tkopp.net, davem@...emloft.net,
 edumazet@...gle.com, kuba@...nel.org, pabeni@...hat.com, corbet@....net,
 kernel@...gutronix.de, linux-can@...r.kernel.org,
 linux-kernel@...r.kernel.org
Subject: Re: [PATCH] j1939: fix unable to send messages with data length zero

On 1/7/25 14:48, Marc Kleine-Budde wrote:
> On 07.01.2025 14:32:17, Alexander Hölzl wrote:
>> The J1939 standard requires the transmission of messages of length 0.
>> For example the proprietary messages are specified with a data length
>> of 0 to 1785. The transmission of such messages was not possible.
>> Sending such a message resulted in no error being returned but no
>> corresponding can frame being generated.
>
> What does your patch do? Please describe it here.
>
> Marc
The patch enables the transmission of zero length J1939 messages.
In order to facilitate this two changes were necessary.

First when the transmission of a new message is requested from userspace
the message is segmented in j1939_sk_send_loop. The segmentation did
account for zero length messages and terminated immediately without
queuing the corresponding skb.

Second when selecting the next skb in j1939_session_skb_get_by_offset to
transmit for a session, it was not checked that there now might be a
zero length skb in the queue.

Also as this is my first real patch I'm submitting please don't hesitate
to tell me if I'm doing something wrong.
>
>> Signed-off-by: Alexander Hölzl <alexander.hoelzl@....net>
>> ---
>>   net/can/j1939/socket.c    | 4 ++--
>>   net/can/j1939/transport.c | 5 +++--
>>   2 files changed, 5 insertions(+), 4 deletions(-)
>>
>> diff --git a/net/can/j1939/socket.c b/net/can/j1939/socket.c
>> index 305dd72c844c..17226b2341d0 100644
>> --- a/net/can/j1939/socket.c
>> +++ b/net/can/j1939/socket.c
>> @@ -1132,7 +1132,7 @@ static int j1939_sk_send_loop(struct j1939_priv *priv,  struct sock *sk,
>>
>>   	todo_size = size;
>>
>> -	while (todo_size) {
>> +	do {
>>   		struct j1939_sk_buff_cb *skcb;
>>
>>   		segment_size = min_t(size_t, J1939_MAX_TP_PACKET_SIZE,
>> @@ -1177,7 +1177,7 @@ static int j1939_sk_send_loop(struct j1939_priv *priv,  struct sock *sk,
>>
>>   		todo_size -= segment_size;
>>   		session->total_queued_size += segment_size;
>> -	}
>> +	} while (todo_size);
>>
>>   	switch (ret) {
>>   	case 0: /* OK */
>> diff --git a/net/can/j1939/transport.c b/net/can/j1939/transport.c
>> index 319f47df3330..99832e60c08d 100644
>> --- a/net/can/j1939/transport.c
>> +++ b/net/can/j1939/transport.c
>> @@ -382,8 +382,9 @@ sk_buff *j1939_session_skb_get_by_offset(struct j1939_session *session,
>>   	skb_queue_walk(&session->skb_queue, do_skb) {
>>   		do_skcb = j1939_skb_to_cb(do_skb);
>>
>> -		if (offset_start >= do_skcb->offset &&
>> -		    offset_start < (do_skcb->offset + do_skb->len)) {
>> +		if ((offset_start >= do_skcb->offset &&
>> +		     offset_start < (do_skcb->offset + do_skb->len)) ||
>> +			(offset_start == 0 && do_skcb->offset == 0 && do_skb->len == 0)) {
>>   			skb = do_skb;
>>   		}
>>   	}
>> --
>> 2.43.0
>>
>>
>>
>