| Message ID | 20241018181842.1368394-8-denkenz@gmail.com |
|---|---|
| State | New |
| Headers | show |
| Series | [RFC,v1,01/10] net: qrtr: ns: validate msglen before ctrl_pkt use | expand |
On 10/18/2024 11:18 AM, Denis Kenzior wrote: > Introduce the ability to bind a QIPCRTR family socket to a specific > endpoint. When a socket is bound, only messages from the bound > endpoint can be received, and any messages sent from the socket are > by default directed to the bound endpoint. Clients can bind a socket > by using the setsockopt system call with the QRTR_BIND_ENDPOINT option > set to the desired endpoint binding. > > A previously set binding can be reset by setting QRTR_BIND_ENDPOINT > option to zero. This behavior matches that of SO_BINDTOIFINDEX. > > This functionality is useful for clients that need to communicate > with a specific device (i.e. endpoint), such as a PCIe-based 5G modem, > and are not interested in messages from other endpoints / nodes. > > Signed-off-by: Denis Kenzior <denkenz@gmail.com> > Reviewed-by: Marcel Holtmann <marcel@holtmann.org> > Reviewed-by: Andy Gross <agross@kernel.org> > --- > include/uapi/linux/qrtr.h | 1 + > net/qrtr/af_qrtr.c | 54 ++++++++++++++++++++++++++++----------- > 2 files changed, 40 insertions(+), 15 deletions(-) > ... > @@ -1313,6 +1331,9 @@ static int qrtr_setsockopt(struct socket *sock, int level, int optname, > case QRTR_REPORT_ENDPOINT: > assign_bit(QRTR_F_REPORT_ENDPOINT, &ipc->flags, val); > break; > + case QRTR_BIND_ENDPOINT: > + ipc->bound_endpoint = val; > + break; > default: > rc = -ENOPROTOOPT; > } > @@ -1346,6 +1367,9 @@ static int qrtr_getsockopt(struct socket *sock, int level, int optname, > case QRTR_REPORT_ENDPOINT: > val = test_bit(QRTR_F_REPORT_ENDPOINT, &ipc->flags); > break; > + case QRTR_BIND_ENDPOINT: > + val = ipc->bound_endpoint; > + break; In the case where an endpoint goes away and a client has bound their socket to an endpoint, would there be any notification to unbind the socket? Is the expectation that the client would get notified through ECONNRESET on the next sendmsg() or receive the BYE/DEL_CLIENT/DEL_SERVER control message. On that cleanup, I guess the client would either re-bind the socket back to 0 or wait for the mhi sysfs to come back and get the new endpoint id? > default: > rc = -ENOPROTOOPT; > }
Hi Chris, >> @@ -1346,6 +1367,9 @@ static int qrtr_getsockopt(struct socket *sock, int >> level, int optname, >> case QRTR_REPORT_ENDPOINT: >> val = test_bit(QRTR_F_REPORT_ENDPOINT, &ipc->flags); >> break; >> + case QRTR_BIND_ENDPOINT: >> + val = ipc->bound_endpoint; >> + break; > > In the case where an endpoint goes away and a client has bound their socket to > an endpoint, would there be any notification to unbind the socket? > I didn't think it was needed. In my use case I would be relying on the relevant device to be removed, (e.g. a udev notification would be received for the devices associated with the PCIe modem). ECONNRESET might also happen, with the udev events following shortly after. > Is the expectation that the client would get notified through ECONNRESET on the > next sendmsg() or receive the BYE/DEL_CLIENT/DEL_SERVER control message. Yes. > > On that cleanup, I guess the client would either re-bind the socket back to 0 or > wait for the mhi sysfs to come back and get the new endpoint id? In my case I would be closing the sockets entirely. But what you describe can also be done. Regards, -Denis
diff --git a/include/uapi/linux/qrtr.h b/include/uapi/linux/qrtr.h index 6d0911984a05..0a8667b049c3 100644 --- a/include/uapi/linux/qrtr.h +++ b/include/uapi/linux/qrtr.h @@ -48,6 +48,7 @@ struct qrtr_ctrl_pkt { /* setsockopt / getsockopt */ #define QRTR_REPORT_ENDPOINT 1 +#define QRTR_BIND_ENDPOINT 2 /* CMSG */ #define QRTR_ENDPOINT 1 diff --git a/net/qrtr/af_qrtr.c b/net/qrtr/af_qrtr.c index 23749a0b0c15..b2f9c25ba8f8 100644 --- a/net/qrtr/af_qrtr.c +++ b/net/qrtr/af_qrtr.c @@ -98,6 +98,7 @@ struct qrtr_sock { struct sockaddr_qrtr us; struct sockaddr_qrtr peer; unsigned long flags; + u32 bound_endpoint; }; static inline struct qrtr_sock *qrtr_sk(struct sock *sk) @@ -587,9 +588,13 @@ int qrtr_endpoint_post(struct qrtr_endpoint *ep, const void *data, size_t len) if (!ipc) goto err; - if (sock_queue_rcv_skb(&ipc->sk, skb)) { - qrtr_port_put(ipc); - goto err; + /* Sockets bound to an endpoint only rx from that endpoint */ + if (!ipc->bound_endpoint || + ipc->bound_endpoint == cb->endpoint_id) { + if (sock_queue_rcv_skb(&ipc->sk, skb)) { + qrtr_port_put(ipc); + goto err; + } } qrtr_port_put(ipc); @@ -928,29 +933,41 @@ static int qrtr_local_enqueue(struct qrtr_node *node, struct sk_buff *skb, { struct qrtr_sock *ipc; struct qrtr_cb *cb; + int ret = -ENODEV; ipc = qrtr_port_lookup(to->sq_port); - if (!ipc || &ipc->sk == skb->sk) { /* do not send to self */ - if (ipc) - qrtr_port_put(ipc); - kfree_skb(skb); - return -ENODEV; - } + if (!ipc) + goto done; + + if (&ipc->sk == skb->sk) /* do not send to self */ + goto done; + + /* + * Filter out unwanted packets that are not on behalf of the bound + * endpoint. Certain special packets (such as an empty NEW_SERVER + * packet that serves as a sentinel value) always go through. + */ + if (endpoint_id && ipc->bound_endpoint && + ipc->bound_endpoint != endpoint_id) + goto done; cb = (struct qrtr_cb *)skb->cb; cb->src_node = from->sq_node; cb->src_port = from->sq_port; cb->endpoint_id = endpoint_id; - if (sock_queue_rcv_skb(&ipc->sk, skb)) { - qrtr_port_put(ipc); - kfree_skb(skb); - return -ENOSPC; - } + ret = -ENOSPC; + if (sock_queue_rcv_skb(&ipc->sk, skb)) + goto done; qrtr_port_put(ipc); return 0; +done: + if (ipc) + qrtr_port_put(ipc); + kfree_skb(skb); + return ret; } /* Queue packet for broadcast. */ @@ -1034,7 +1051,8 @@ static int qrtr_sendmsg(struct socket *sock, struct msghdr *msg, size_t len) } else if (addr->sq_node == ipc->us.sq_node) { enqueue_fn = qrtr_local_enqueue; } else { - endpoint_id = msg_endpoint_id; + endpoint_id = msg_endpoint_id ? + msg_endpoint_id : ipc->bound_endpoint; node = qrtr_node_lookup(endpoint_id, addr->sq_node); if (!node) { @@ -1313,6 +1331,9 @@ static int qrtr_setsockopt(struct socket *sock, int level, int optname, case QRTR_REPORT_ENDPOINT: assign_bit(QRTR_F_REPORT_ENDPOINT, &ipc->flags, val); break; + case QRTR_BIND_ENDPOINT: + ipc->bound_endpoint = val; + break; default: rc = -ENOPROTOOPT; } @@ -1346,6 +1367,9 @@ static int qrtr_getsockopt(struct socket *sock, int level, int optname, case QRTR_REPORT_ENDPOINT: val = test_bit(QRTR_F_REPORT_ENDPOINT, &ipc->flags); break; + case QRTR_BIND_ENDPOINT: + val = ipc->bound_endpoint; + break; default: rc = -ENOPROTOOPT; }