| Message ID | 20240806154324.40764-1-donald.hunter@gmail.com (mailing list archive) |
|---|---|
| State | Awaiting Upstream |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | [nf,v1] netfilter: nfnetlink: Initialise extack before use in ACKs | expand |
On Tue, Aug 06, 2024 at 04:43:24PM +0100, Donald Hunter wrote: > Add missing extack initialisation when ACKing BATCH_BEGIN and BATCH_END. > > Fixes: bf2ac490d28c ("netfilter: nfnetlink: Handle ACK flags for batch messages") > Signed-off-by: Donald Hunter <donald.hunter@gmail.com> Hi Donald, I see two other places that extack is used in nfnetlink_rcv_batch(). Is it safe to leave them as-is? > --- > net/netfilter/nfnetlink.c | 5 ++++- > 1 file changed, 4 insertions(+), 1 deletion(-) > > diff --git a/net/netfilter/nfnetlink.c b/net/netfilter/nfnetlink.c > index 4abf660c7baf..932b3ddb34f1 100644 > --- a/net/netfilter/nfnetlink.c > +++ b/net/netfilter/nfnetlink.c > @@ -427,8 +427,10 @@ static void nfnetlink_rcv_batch(struct sk_buff *skb, struct nlmsghdr *nlh, > > nfnl_unlock(subsys_id); > > - if (nlh->nlmsg_flags & NLM_F_ACK) > + if (nlh->nlmsg_flags & NLM_F_ACK) { > + memset(&extack, 0, sizeof(extack)); > nfnl_err_add(&err_list, nlh, 0, &extack); > + } > > while (skb->len >= nlmsg_total_size(0)) { > int msglen, type; > @@ -577,6 +579,7 @@ static void nfnetlink_rcv_batch(struct sk_buff *skb, struct nlmsghdr *nlh, > ss->abort(net, oskb, NFNL_ABORT_NONE); > netlink_ack(oskb, nlmsg_hdr(oskb), err, NULL); > } else if (nlh->nlmsg_flags & NLM_F_ACK) { > + memset(&extack, 0, sizeof(extack)); > nfnl_err_add(&err_list, nlh, 0, &extack); > } > } else { > -- > 2.45.2 > >
Simon Horman <horms@kernel.org> writes: > On Tue, Aug 06, 2024 at 04:43:24PM +0100, Donald Hunter wrote: >> Add missing extack initialisation when ACKing BATCH_BEGIN and BATCH_END. >> >> Fixes: bf2ac490d28c ("netfilter: nfnetlink: Handle ACK flags for batch messages") >> Signed-off-by: Donald Hunter <donald.hunter@gmail.com> > > Hi Donald, > > I see two other places that extack is used in nfnetlink_rcv_batch(). > Is it safe to leave them as-is? There is a memset at the start of the main while loop that zeroes extack for those two cases.
On Fri, Aug 09, 2024 at 12:15:55PM +0100, Donald Hunter wrote: > Simon Horman <horms@kernel.org> writes: > > > On Tue, Aug 06, 2024 at 04:43:24PM +0100, Donald Hunter wrote: > >> Add missing extack initialisation when ACKing BATCH_BEGIN and BATCH_END. > >> > >> Fixes: bf2ac490d28c ("netfilter: nfnetlink: Handle ACK flags for batch messages") > >> Signed-off-by: Donald Hunter <donald.hunter@gmail.com> > > > > Hi Donald, > > > > I see two other places that extack is used in nfnetlink_rcv_batch(). > > Is it safe to leave them as-is? > > There is a memset at the start of the main while loop that zeroes extack > for those two cases. Thanks Donald, I missed that. I was wondering if it might be best to clear extack at the beginning of the function. But if the loop needs to clear it on each iteration, then I think your solution is good. Reviewed-by: Simon Horman <horms@kernel.org>
diff --git a/net/netfilter/nfnetlink.c b/net/netfilter/nfnetlink.c index 4abf660c7baf..932b3ddb34f1 100644 --- a/net/netfilter/nfnetlink.c +++ b/net/netfilter/nfnetlink.c @@ -427,8 +427,10 @@ static void nfnetlink_rcv_batch(struct sk_buff *skb, struct nlmsghdr *nlh, nfnl_unlock(subsys_id); - if (nlh->nlmsg_flags & NLM_F_ACK) + if (nlh->nlmsg_flags & NLM_F_ACK) { + memset(&extack, 0, sizeof(extack)); nfnl_err_add(&err_list, nlh, 0, &extack); + } while (skb->len >= nlmsg_total_size(0)) { int msglen, type; @@ -577,6 +579,7 @@ static void nfnetlink_rcv_batch(struct sk_buff *skb, struct nlmsghdr *nlh, ss->abort(net, oskb, NFNL_ABORT_NONE); netlink_ack(oskb, nlmsg_hdr(oskb), err, NULL); } else if (nlh->nlmsg_flags & NLM_F_ACK) { + memset(&extack, 0, sizeof(extack)); nfnl_err_add(&err_list, nlh, 0, &extack); } } else {
Add missing extack initialisation when ACKing BATCH_BEGIN and BATCH_END. Fixes: bf2ac490d28c ("netfilter: nfnetlink: Handle ACK flags for batch messages") Signed-off-by: Donald Hunter <donald.hunter@gmail.com> --- net/netfilter/nfnetlink.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-)