| Message ID | 1440367577-15786-2-git-send-email-christophe-h.ricard@st.com |
|---|---|
| State | RFC, archived |
| Delegated to: | David Miller |
| Headers | show |
On Sun, Aug 23, 2015 at 3:06 PM, Christophe Ricard <christophe.ricard@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.ricard@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 majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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; + } skb = netlink_alloc_skb(in_skb->sk, nlmsg_total_size(payload), NETLINK_CB(in_skb).portid, GFP_KERNEL); @@ -2898,7 +2906,8 @@ void netlink_ack(struct sk_buff *in_skb, struct nlmsghdr *nlh, int err) NLMSG_ERROR, payload, 0); errmsg = nlmsg_data(rep); errmsg->error = err; - memcpy(&errmsg->msg, nlh, err ? nlh->nlmsg_len : sizeof(*nlh)); + + memcpy(&errmsg->msg, nlh, err ? size : sizeof(*nlh)); netlink_unicast(in_skb->sk, skb, NETLINK_CB(in_skb).portid, MSG_DONTWAIT); } EXPORT_SYMBOL(netlink_ack);
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.ricard@st.com> --- net/netlink/af_netlink.c | 17 +++++++++++++---- 1 file changed, 13 insertions(+), 4 deletions(-)