Re: [PATCH v1 3/4] j1939: socket: j1939_sk_recv_one(): Fix matching if src_name is set but incoming packet's dst_name is unset

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 





On 24.01.19 08:44, Kurt Van Dijck wrote:
On wo, 23 jan 2019 13:01:13 +0100, Oleksij Rempel wrote:
We only want to receive the package if src_name name packet's dest_name
is set and they are equal. If one name is missing we compare the
addresses instead (else branch).

Signed-off-by: Oleksij Rempel <o.rempel@xxxxxxxxxxxxxx>
---
  net/can/j1939/socket.c | 5 ++---
  1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/net/can/j1939/socket.c b/net/can/j1939/socket.c
index 73ac92163373..9d3da573c31d 100644
--- a/net/can/j1939/socket.c
+++ b/net/can/j1939/socket.c
@@ -134,9 +134,8 @@ static void j1939_sk_recv_one(struct j1939_sock *jsk, struct sk_buff *oskb)
  		/* this socket does not take packets from this iface */
  		return;
  	if (!(jsk->state & J1939_SOCK_PROMISC)) {
BTW, I assumed you had dropped promiscuous operation completely?

why?

-		if (jsk->addr.src_name) {

if src_name is set, then the user want to be addressed using its name.

-			if (oskcb->addr.dst_name &&

dst_name is set regardless of what the (receiving) user wants, it is set
on available name.

So this test actually is the good one, you don't want to fallback to
addr comparison because you have not (yet) successfully claimed the
addr, and hence, the recv part does not assign dst_name (yet).

IMHO, you're breaking reception path during the address claiming
process.
Can you explain what problem you tried to solve here?


bind() with NAME no SA is set,
run AC and get SA 0x80

get packet to 0x80, AC cache will interpolate NAME from address.

so, packet will be passed.

get packet to 0x81, no name was found in AC cache

packet will be passed -- this looks wrong to me. Why should we pass packets with different destination? Especially if socket was not configured to do so.

If you wont to run AC and normal operation on same socket, then:
bind() with NAME
set J1939_SOCK_PROMISC
set filters
after AC is done
remove filters
remove J1939_SOCK_PROMISC
re-bind with NAME and ADDR

Well this work flow, looks wrong as well.



-			    oskcb->addr.dst_name != jsk->addr.src_name)
+		if (jsk->addr.src_name && oskcb->addr.dst_name) {
+			if (oskcb->addr.dst_name != jsk->addr.src_name)

Kind regards,
kurt



Kind regards,
Oleksij Rempel

--
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |



[Index of Archives]     [Automotive Discussions]     [Linux ARM Kernel]     [Linux ARM]     [Linux Omap]     [Fedora ARM]     [IETF Annouce]     [Security]     [Bugtraq]     [Linux]     [Linux OMAP]     [Linux MIPS]     [eCos]     [Asterisk Internet PBX]     [Linux API]     [CAN Bus]

  Powered by Linux