| Message ID | 20241018181842.1368394-9-denkenz@gmail.com (mailing list archive) |
|---|---|
| State | RFC |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | QRTR Multi-endpoint support | expand |
On 10/18/2024 11:18 AM, Denis Kenzior wrote: > These messages are explicitly filtered out by the in-kernel name > service (ns.c). Filter them out even earlier to save some CPU cycles. > > Signed-off-by: Denis Kenzior <denkenz@gmail.com> > Reviewed-by: Marcel Holtmann <marcel@holtmann.org> > Reviewed-by: Andy Gross <agross@kernel.org> > --- > net/qrtr/af_qrtr.c | 5 +++++ > 1 file changed, 5 insertions(+) > > diff --git a/net/qrtr/af_qrtr.c b/net/qrtr/af_qrtr.c > index b2f9c25ba8f8..95c9679725ee 100644 > --- a/net/qrtr/af_qrtr.c > +++ b/net/qrtr/af_qrtr.c > @@ -560,6 +560,11 @@ int qrtr_endpoint_post(struct qrtr_endpoint *ep, const void *data, size_t len) > if (!size || len != ALIGN(size, 4) + hdrlen) > goto err; > > + /* Don't allow remote lookups */ > + if (cb->type == QRTR_TYPE_NEW_LOOKUP || > + cb->type == QRTR_TYPE_DEL_LOOKUP) > + goto err; > + Just curious, was this case observed? I thought we blocked clients from sending this control message to remotes and I didnt think the ns broadcasts it either. > if ((cb->type == QRTR_TYPE_NEW_SERVER || > cb->type == QRTR_TYPE_RESUME_TX) && > size < sizeof(struct qrtr_ctrl_pkt))
Hi Chris, >> @@ -560,6 +560,11 @@ int qrtr_endpoint_post(struct qrtr_endpoint *ep, const >> void *data, size_t len) >> if (!size || len != ALIGN(size, 4) + hdrlen) >> goto err; >> + /* Don't allow remote lookups */ >> + if (cb->type == QRTR_TYPE_NEW_LOOKUP || >> + cb->type == QRTR_TYPE_DEL_LOOKUP) >> + goto err; >> + > > Just curious, was this case observed? I thought we blocked clients from sending > this control message to remotes and I didnt think the ns broadcasts it either. No I didn't see this in practice, so this patch is not strictly necessary. One thing I thought about originally was to remove the check in ns.c in order to extend struct qrtr_lookup with the endpoint id: an application interested only in services on a certain endpoint could send NEW_LOOKUP with the endpoint id included as a CMSG header. It made the proposal more complicated though and I didn't think it was really needed since QRTR_BIND_ENDPOINT was the better tool for this kind of use case. Regards, -Denis
diff --git a/net/qrtr/af_qrtr.c b/net/qrtr/af_qrtr.c index b2f9c25ba8f8..95c9679725ee 100644 --- a/net/qrtr/af_qrtr.c +++ b/net/qrtr/af_qrtr.c @@ -560,6 +560,11 @@ int qrtr_endpoint_post(struct qrtr_endpoint *ep, const void *data, size_t len) if (!size || len != ALIGN(size, 4) + hdrlen) goto err; + /* Don't allow remote lookups */ + if (cb->type == QRTR_TYPE_NEW_LOOKUP || + cb->type == QRTR_TYPE_DEL_LOOKUP) + goto err; + if ((cb->type == QRTR_TYPE_NEW_SERVER || cb->type == QRTR_TYPE_RESUME_TX) && size < sizeof(struct qrtr_ctrl_pkt))