* [PATCH for-4.14-rc] RDMA/netlink: Restore nlmsg_len calculation in ibnl_put_attr
@ 2017-09-28 11:49 Shiraz Saleem
[not found] ` <20170928114953.9600-1-shiraz.saleem-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
0 siblings, 1 reply; 4+ messages in thread
From: Shiraz Saleem @ 2017-09-28 11:49 UTC (permalink / raw)
To: dledford-H+wXaHxf7aLQT0dZR+AlfA
Cc: linux-rdma-u79uwXL29TY76Z2rM5mHXA,
e1000-rdma-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f,
leon-DgEjT+Ai2ygdnm+yROfE0A, Shiraz Saleem
Commit 1a1c116f3dcf removes nlmsg_len calculation in
ibnl_put_attr causing netlink messages to be rejected due
to incorrect length.
Restore the netlink message header length calculation
to include the added attribute.
Fixes: 1a1c116f3dcf ("RDMA/netlink: Simplify the put_msg and put_attr")
Signed-off-by: Shiraz Saleem <shiraz.saleem-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
Signed-off-by: Tatyana Nikolova <tatyana.e.nikolova-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
---
drivers/infiniband/core/netlink.c | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/drivers/infiniband/core/netlink.c b/drivers/infiniband/core/netlink.c
index e685148..889d091 100644
--- a/drivers/infiniband/core/netlink.c
+++ b/drivers/infiniband/core/netlink.c
@@ -150,10 +150,14 @@ EXPORT_SYMBOL(ibnl_put_msg);
int ibnl_put_attr(struct sk_buff *skb, struct nlmsghdr *nlh,
int len, void *data, int type)
{
+ unsigned char *prev_tail;
+
+ prev_tail = skb_tail_pointer(skb);
if (nla_put(skb, type, len, data)) {
nlmsg_cancel(skb, nlh);
return -EMSGSIZE;
}
+ nlh->nlmsg_len += skb_tail_pointer(skb) - prev_tail;
return 0;
}
EXPORT_SYMBOL(ibnl_put_attr);
--
2.8.3
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH for-4.14-rc] RDMA/netlink: Restore nlmsg_len calculation in ibnl_put_attr
[not found] ` <20170928114953.9600-1-shiraz.saleem-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
@ 2017-09-28 13:24 ` Leon Romanovsky
[not found] ` <20170928132457.GW2297-U/DQcQFIOTAAJjI8aNfphQ@public.gmane.org>
0 siblings, 1 reply; 4+ messages in thread
From: Leon Romanovsky @ 2017-09-28 13:24 UTC (permalink / raw)
To: Shiraz Saleem
Cc: dledford-H+wXaHxf7aLQT0dZR+AlfA,
linux-rdma-u79uwXL29TY76Z2rM5mHXA,
e1000-rdma-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f
[-- Attachment #1: Type: text/plain, Size: 1702 bytes --]
On Thu, Sep 28, 2017 at 06:49:53AM -0500, Shiraz Saleem wrote:
> Commit 1a1c116f3dcf removes nlmsg_len calculation in
> ibnl_put_attr causing netlink messages to be rejected due
> to incorrect length.
>
> Restore the netlink message header length calculation
> to include the added attribute.
>
> Fixes: 1a1c116f3dcf ("RDMA/netlink: Simplify the put_msg and put_attr")
> Signed-off-by: Shiraz Saleem <shiraz.saleem-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
> Signed-off-by: Tatyana Nikolova <tatyana.e.nikolova-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
> ---
> drivers/infiniband/core/netlink.c | 4 ++++
> 1 file changed, 4 insertions(+)
>
The length supposed to be updated in ibnl_put_msg, where you should
supply correct length from the beginning. The suggested way to supply
length for unknown data is to provide NLMSG_DEFAULT_SIZE while allocating
new netlink message.
NLMSG_DEFAULT_SIZE ensures that netlink fits into one page.
It is better to avoid messing with message length after allocations,
especially for the attributes.
Thanks
> diff --git a/drivers/infiniband/core/netlink.c b/drivers/infiniband/core/netlink.c
> index e685148..889d091 100644
> --- a/drivers/infiniband/core/netlink.c
> +++ b/drivers/infiniband/core/netlink.c
> @@ -150,10 +150,14 @@ EXPORT_SYMBOL(ibnl_put_msg);
> int ibnl_put_attr(struct sk_buff *skb, struct nlmsghdr *nlh,
> int len, void *data, int type)
> {
> + unsigned char *prev_tail;
> +
> + prev_tail = skb_tail_pointer(skb);
> if (nla_put(skb, type, len, data)) {
> nlmsg_cancel(skb, nlh);
> return -EMSGSIZE;
> }
> + nlh->nlmsg_len += skb_tail_pointer(skb) - prev_tail;
> return 0;
> }
> EXPORT_SYMBOL(ibnl_put_attr);
> --
> 2.8.3
>
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH for-4.14-rc] RDMA/netlink: Restore nlmsg_len calculation in ibnl_put_attr
[not found] ` <20170928132457.GW2297-U/DQcQFIOTAAJjI8aNfphQ@public.gmane.org>
@ 2017-09-28 19:00 ` Shiraz Saleem
[not found] ` <20170928190033.GA12760-GOXS9JX10wfOxmVO0tvppfooFf0ArEBIu+b9c/7xato@public.gmane.org>
0 siblings, 1 reply; 4+ messages in thread
From: Shiraz Saleem @ 2017-09-28 19:00 UTC (permalink / raw)
To: Leon Romanovsky
Cc: dledford-H+wXaHxf7aLQT0dZR+AlfA,
linux-rdma-u79uwXL29TY76Z2rM5mHXA,
e1000-rdma-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f
On Thu, Sep 28, 2017 at 04:24:57PM +0300, Leon Romanovsky wrote:
> On Thu, Sep 28, 2017 at 06:49:53AM -0500, Shiraz Saleem wrote:
> > Commit 1a1c116f3dcf removes nlmsg_len calculation in
> > ibnl_put_attr causing netlink messages to be rejected due
> > to incorrect length.
> >
> > Restore the netlink message header length calculation
> > to include the added attribute.
> >
> > Fixes: 1a1c116f3dcf ("RDMA/netlink: Simplify the put_msg and put_attr")
> > Signed-off-by: Shiraz Saleem <shiraz.saleem-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
> > Signed-off-by: Tatyana Nikolova <tatyana.e.nikolova-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
> > ---
> > drivers/infiniband/core/netlink.c | 4 ++++
> > 1 file changed, 4 insertions(+)
> >
>
> The length supposed to be updated in ibnl_put_msg, where you should
> supply correct length from the beginning. The suggested way to supply
> length for unknown data is to provide NLMSG_DEFAULT_SIZE while allocating
> new netlink message.
>
> NLMSG_DEFAULT_SIZE ensures that netlink fits into one page.
>
> It is better to avoid messing with message length after allocations,
> especially for the attributes.
>
Hi Leon -
It is not neccessary that the length is passed in ibnl_put_msg.
See drivers/infiniband/core/addr.c/ib_nl_ip_send_msg().
In ib_nl_ip_send_msg() nlmsg_len is corrected using nlmsg_end,
which is typically called after attributes are added to calculate
the correct length.
The _original_ ibnl_put_attr wrapper in netlink.c achieves the same
by calculating the nlmsg_len.
One can argue that it is undesirable to calculate nlmsg_len on
every ibnl_put_attr call. An alternate fix we could do is instead of
recalculating the length with every ibnl_put_attr call is to
add a nlmsg_end after the last ibnl_put_attr call.
Do you agree this is a reasonable solution to fix your commit?
Shiraz
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH for-4.14-rc] RDMA/netlink: Restore nlmsg_len calculation in ibnl_put_attr
[not found] ` <20170928190033.GA12760-GOXS9JX10wfOxmVO0tvppfooFf0ArEBIu+b9c/7xato@public.gmane.org>
@ 2017-09-28 19:47 ` Leon Romanovsky
0 siblings, 0 replies; 4+ messages in thread
From: Leon Romanovsky @ 2017-09-28 19:47 UTC (permalink / raw)
To: Shiraz Saleem
Cc: dledford-H+wXaHxf7aLQT0dZR+AlfA,
linux-rdma-u79uwXL29TY76Z2rM5mHXA,
e1000-rdma-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f
[-- Attachment #1: Type: text/plain, Size: 2294 bytes --]
On Thu, Sep 28, 2017 at 02:00:33PM -0500, Shiraz Saleem wrote:
> On Thu, Sep 28, 2017 at 04:24:57PM +0300, Leon Romanovsky wrote:
> > On Thu, Sep 28, 2017 at 06:49:53AM -0500, Shiraz Saleem wrote:
> > > Commit 1a1c116f3dcf removes nlmsg_len calculation in
> > > ibnl_put_attr causing netlink messages to be rejected due
> > > to incorrect length.
> > >
> > > Restore the netlink message header length calculation
> > > to include the added attribute.
> > >
> > > Fixes: 1a1c116f3dcf ("RDMA/netlink: Simplify the put_msg and put_attr")
> > > Signed-off-by: Shiraz Saleem <shiraz.saleem-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
> > > Signed-off-by: Tatyana Nikolova <tatyana.e.nikolova-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
> > > ---
> > > drivers/infiniband/core/netlink.c | 4 ++++
> > > 1 file changed, 4 insertions(+)
> > >
> >
> > The length supposed to be updated in ibnl_put_msg, where you should
> > supply correct length from the beginning. The suggested way to supply
> > length for unknown data is to provide NLMSG_DEFAULT_SIZE while allocating
> > new netlink message.
> >
> > NLMSG_DEFAULT_SIZE ensures that netlink fits into one page.
> >
> > It is better to avoid messing with message length after allocations,
> > especially for the attributes.
> >
>
> Hi Leon -
>
> It is not neccessary that the length is passed in ibnl_put_msg.
>
> See drivers/infiniband/core/addr.c/ib_nl_ip_send_msg().
>
> In ib_nl_ip_send_msg() nlmsg_len is corrected using nlmsg_end,
> which is typically called after attributes are added to calculate
> the correct length.
>
> The _original_ ibnl_put_attr wrapper in netlink.c achieves the same
> by calculating the nlmsg_len.
>
> One can argue that it is undesirable to calculate nlmsg_len on
> every ibnl_put_attr call. An alternate fix we could do is instead of
> recalculating the length with every ibnl_put_attr call is to
> add a nlmsg_end after the last ibnl_put_attr call.
>
> Do you agree this is a reasonable solution to fix your commit?
As long as you don't add it into ibnl_put_attr and add nlmsg_end into the caller sites,
I'm more than fine with that.
The rationale behind my "simplify ..." commit was to get rid of ibnl_put_attr
and replace all calls to appropriate nla_* calls which performs type
checking.
Thanks
>
> Shiraz
>
>
>
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2017-09-28 19:47 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-09-28 11:49 [PATCH for-4.14-rc] RDMA/netlink: Restore nlmsg_len calculation in ibnl_put_attr Shiraz Saleem
[not found] ` <20170928114953.9600-1-shiraz.saleem-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
2017-09-28 13:24 ` Leon Romanovsky
[not found] ` <20170928132457.GW2297-U/DQcQFIOTAAJjI8aNfphQ@public.gmane.org>
2017-09-28 19:00 ` Shiraz Saleem
[not found] ` <20170928190033.GA12760-GOXS9JX10wfOxmVO0tvppfooFf0ArEBIu+b9c/7xato@public.gmane.org>
2017-09-28 19:47 ` Leon Romanovsky
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.