[PATCH] isotp: do not validate RX address when the broadcast flag is active

[PATCH] isotp: do not validate RX address when the broadcast flag is active

[Thomseeen]

EADDRNOTAVAIL shouldn't be thrown when an invalid RX address (e.g. NO_CAN_ID)
is set while the socket is used with the CAN_ISOTP_SF_BROADCAST flag.

Signed-off-by: Thomseeen <thwa1@web.de>
---
 net/can/isotp.c | 21 +++++++++++++--------
 1 file changed, 13 insertions(+), 8 deletions(-)

diff --git a/net/can/isotp.c b/net/can/isotp.c
index fb6dba1..ae939bf 100644
--- a/net/can/isotp.c
+++ b/net/can/isotp.c
@@ -1106,20 +1106,25 @@ static int isotp_bind(struct socket *sock, struct sockaddr *uaddr, int len)
 #endif
 		return -EINVAL;

-	if (addr->can_addr.tp.rx_id == addr->can_addr.tp.tx_id)
-		return -EADDRNOTAVAIL;
+	/* do not register frame reception for functional addressing */
+	if (so->opt.flags & CAN_ISOTP_SF_BROADCAST)
+		do_rx_reg = 0;
+
+	/* do not validate rx address for functional addressing */
+	if (do_rx_reg) {
+		if (addr->can_addr.tp.rx_id == addr->can_addr.tp.tx_id)
+			return -EADDRNOTAVAIL;

-	if ((addr->can_addr.tp.rx_id | addr->can_addr.tp.tx_id) &
-	    (CAN_ERR_FLAG | CAN_RTR_FLAG))
+		if (addr->can_addr.tp.rx_id & (CAN_ERR_FLAG | CAN_RTR_FLAG))
+			return -EADDRNOTAVAIL;
+	}
+
+	if (addr->can_addr.tp.tx_id & (CAN_ERR_FLAG | CAN_RTR_FLAG))
 		return -EADDRNOTAVAIL;

 	if (!addr->can_ifindex)
 		return -ENODEV;

-	/* do not register frame reception for functional addressing */
-	if (so->opt.flags & CAN_ISOTP_SF_BROADCAST)
-		do_rx_reg = 0;
-
 	lock_sock(sk);

 	if (so->bound && addr->can_ifindex == so->ifindex &&
--
2.25.1

Re: [PATCH] isotp: do not validate RX address when the broadcast flag is active

[Marc Kleine-Budde]

[-- Attachment #1.1: Type: text/plain, Size: 731 bytes --]

On 12/4/20 2:55 PM, Thomseeen wrote:
> EADDRNOTAVAIL shouldn't be thrown when an invalid RX address (e.g. NO_CAN_ID)
> is set while the socket is used with the CAN_ISOTP_SF_BROADCAST flag.
> 
> Signed-off-by: Thomseeen <thwa1@web.de>

Is this your realname? For the kernel the s-o-b requires your realname. For
details about the S-o-b, see:

https://elixir.bootlin.com/linux/v5.9.12/source/Documentation/process/submitting-patches.rst#L423

Marc

-- 
Pengutronix e.K.                 | Marc Kleine-Budde           |
Embedded Linux                   | https://www.pengutronix.de  |
Vertretung West/Dortmund         | Phone: +49-231-2826-924     |
Amtsgericht Hildesheim, HRA 2686 | Fax:   +49-5121-206917-5555 |


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH] isotp: do not validate RX address when the broadcast flag is active

[Oliver Hartkopp]

On 05.12.20 21:59, Marc Kleine-Budde wrote:
> On 12/4/20 2:55 PM, Thomseeen wrote:
>> EADDRNOTAVAIL shouldn't be thrown when an invalid RX address (e.g. NO_CAN_ID)
>> is set while the socket is used with the CAN_ISOTP_SF_BROADCAST flag.
>>
>> Signed-off-by: Thomseeen <thwa1@web.de>
> 
> Is this your realname? For the kernel the s-o-b requires your realname. For
> details about the S-o-b, see:
> 
> https://elixir.bootlin.com/linux/v5.9.12/source/Documentation/process/submitting-patches.rst#L423
> 

Thomas' name is correct in my patch here:
https://lore.kernel.org/linux-can/ce547683-925d-6971-6566-a0b54146090a@pengutronix.de/T/#m3c780406ff756cd0b66f1d34744fb4d8eb4abaf6

Regards,
Oliver

Re: [PATCH] isotp: do not validate RX address when the broadcast flag is active

[Oliver Hartkopp]

On 04.12.20 14:55, Thomseeen wrote:
> EADDRNOTAVAIL shouldn't be thrown when an invalid RX address (e.g. NO_CAN_ID)
> is set while the socket is used with the CAN_ISOTP_SF_BROADCAST flag.
> 
> Signed-off-by: Thomseeen <thwa1@web.de>

Acked-by: Oliver Hartkopp <socketcan@hartkopp.net>

> ---
>   net/can/isotp.c | 21 +++++++++++++--------
>   1 file changed, 13 insertions(+), 8 deletions(-)
> 
> diff --git a/net/can/isotp.c b/net/can/isotp.c
> index fb6dba1..ae939bf 100644
> --- a/net/can/isotp.c
> +++ b/net/can/isotp.c
> @@ -1106,20 +1106,25 @@ static int isotp_bind(struct socket *sock, struct sockaddr *uaddr, int len)
>   #endif
>   		return -EINVAL;
> 
> -	if (addr->can_addr.tp.rx_id == addr->can_addr.tp.tx_id)
> -		return -EADDRNOTAVAIL;
> +	/* do not register frame reception for functional addressing */
> +	if (so->opt.flags & CAN_ISOTP_SF_BROADCAST)
> +		do_rx_reg = 0;
> +
> +	/* do not validate rx address for functional addressing */
> +	if (do_rx_reg) {
> +		if (addr->can_addr.tp.rx_id == addr->can_addr.tp.tx_id)
> +			return -EADDRNOTAVAIL;
> 
> -	if ((addr->can_addr.tp.rx_id | addr->can_addr.tp.tx_id) &
> -	    (CAN_ERR_FLAG | CAN_RTR_FLAG))
> +		if (addr->can_addr.tp.rx_id & (CAN_ERR_FLAG | CAN_RTR_FLAG))
> +			return -EADDRNOTAVAIL;
> +	}
> +
> +	if (addr->can_addr.tp.tx_id & (CAN_ERR_FLAG | CAN_RTR_FLAG))
>   		return -EADDRNOTAVAIL;
> 
>   	if (!addr->can_ifindex)
>   		return -ENODEV;
> 
> -	/* do not register frame reception for functional addressing */
> -	if (so->opt.flags & CAN_ISOTP_SF_BROADCAST)
> -		do_rx_reg = 0;
> -
>   	lock_sock(sk);
> 
>   	if (so->bound && addr->can_ifindex == so->ifindex &&
> --
> 2.25.1
> 

RE: [PATCH] isotp: do not validate RX address when the broadcast flag is active

[thwa1]

Hi,

On 05.12.20 21:59, Marc Kleine-Budde wrote:
> On 12/4/20 2:55 PM, Thomseeen wrote:
> > EADDRNOTAVAIL shouldn't be thrown when an invalid RX address (e.g. NO_CAN_ID)
> > is set while the socket is used with the CAN_ISOTP_SF_BROADCAST flag.
> >
> > Signed-off-by: Thomseeen <thwa1@web.de>
>
> Is this your realname? For the kernel the s-o-b requires your realname. [...]

sorry, it is of course not. Messed up with my git configs. It should be:
Signed-off-by: Thomas Wagner <thwa1@web.de>
Should I repost the patch with a proper tag?

Regards
Thomas

Re: [PATCH] isotp: do not validate RX address when the broadcast flag is active

[Marc Kleine-Budde]

[-- Attachment #1.1: Type: text/plain, Size: 953 bytes --]

On 12/5/20 10:15 PM, Oliver Hartkopp wrote:
> On 04.12.20 14:55, Thomseeen wrote:
>> EADDRNOTAVAIL shouldn't be thrown when an invalid RX address (e.g. NO_CAN_ID)
>> is set while the socket is used with the CAN_ISOTP_SF_BROADCAST flag.
>>
>> Signed-off-by: Thomseeen <thwa1@web.de>
> 
> Acked-by: Oliver Hartkopp <socketcan@hartkopp.net>

Should this be squashed into "support for functional addressing"?

Can you re-post the two or three isotp patches, as they should go into
can-next/net-next?

> 98bef9295d07 can: isotp: isotp_setsockopt(): block setsockopt on bound sockets
> 21f10f1b5c3a can: isotp: add SF_BROADCAST support for functional addressing

Marc

-- 
Pengutronix e.K.                 | Marc Kleine-Budde           |
Embedded Linux                   | https://www.pengutronix.de  |
Vertretung West/Dortmund         | Phone: +49-231-2826-924     |
Amtsgericht Hildesheim, HRA 2686 | Fax:   +49-5121-206917-5555 |


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH] isotp: do not validate RX address when the broadcast flag is active

[Oliver Hartkopp]

On 05.12.20 22:23, Marc Kleine-Budde wrote:
> On 12/5/20 10:15 PM, Oliver Hartkopp wrote:
>> On 04.12.20 14:55, Thomseeen wrote:
>>> EADDRNOTAVAIL shouldn't be thrown when an invalid RX address (e.g. NO_CAN_ID)
>>> is set while the socket is used with the CAN_ISOTP_SF_BROADCAST flag.
>>>
>>> Signed-off-by: Thomseeen <thwa1@web.de>
>>
>> Acked-by: Oliver Hartkopp <socketcan@hartkopp.net>
> 
> Should this be squashed into "support for functional addressing"?
> 
> Can you re-post the two or three isotp patches, as they should go into
> can-next/net-next?
> 

Yes, will do.

>> 98bef9295d07 can: isotp: isotp_setsockopt(): block setsockopt on bound sockets
>> 21f10f1b5c3a can: isotp: add SF_BROADCAST support for functional addressing
> 
> Marc
>