On Sun, Aug 23, 2015 at 3:06 PM, Christophe Ricard
<christophe.ric...@gmail.com> wrote:
> Currently, ACK in case of error contains a full copy of the originating
> message. This can cause lost ACKs with large netlink messages, especially
> after commit c05cdb1b864f ("netlink: allow large data transfers from
> user-space").
>
> Send back a capped message instead.
>
> Signed-off-by: Christophe Ricard <christophe-h.ric...@st.com>
> ---
>  net/netlink/af_netlink.c | 17 +++++++++++++----
>  1 file changed, 13 insertions(+), 4 deletions(-)
>
> diff --git a/net/netlink/af_netlink.c b/net/netlink/af_netlink.c
> index 67d2104..9df862c 100644
> --- a/net/netlink/af_netlink.c
> +++ b/net/netlink/af_netlink.c
> @@ -85,6 +85,9 @@ struct listeners {
>  #define NETLINK_F_RECV_NO_ENOBUFS      0x8
>  #define NETLINK_F_LISTEN_ALL_NSID      0x10
>
> +/* Arbitrary value for a small message less than PAGE_SIZE */
> +#define NETLINK_ERR_MESSAGE_CAP        128
> +
>  static inline int netlink_is_kernel(struct sock *sk)
>  {
>         return nlk_sk(sk)->flags & NETLINK_F_KERNEL_SOCKET;
> @@ -2873,10 +2876,15 @@ void netlink_ack(struct sk_buff *in_skb, struct 
> nlmsghdr *nlh, int err)
>         struct nlmsghdr *rep;
>         struct nlmsgerr *errmsg;
>         size_t payload = sizeof(*errmsg);
> +       size_t size = 0;
>
> -       /* error messages get the original request appened */
> -       if (err)
> -               payload += nlmsg_len(nlh);
> +       /* error messages get a cap request appened */
> +       if (err) {
> +               payload += nlmsg_len(nlh) > NETLINK_ERR_MESSAGE_CAP ?
> +                          NETLINK_ERR_MESSAGE_CAP : nlmsg_len(nlh);
> +               size = nlmsg_len(nlh) > NETLINK_ERR_MESSAGE_CAP ?
> +                     (NETLINK_ERR_MESSAGE_CAP + NLMSG_HDRLEN) : 
> nlh->nlmsg_len;
> +       }

This arbitrary cap can truncate the msg at unfortunate boundaries,
leaving nests open or slicing a TLV attr in half.  So the receiver app
now needs to handle those cases or ignore the payload.  But if
ignoring the payload, might as well not include the payload in the
first place, using the flag idea talked about earlier.
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to