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@xxxxxxxxx>
> > Signed-off-by: Tatyana Nikolova <tatyana.e.nikolova@xxxxxxxxx>
> > ---
> > 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@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
