Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider: - [Low] netfilter: ebt_snat backport notes misdescribe the tree (skb_make_writable) --
commit 104d376b9788e858d4ac4df6973edf7b2248c98a Author: Yiming Qian <[email protected]> ms/netfilter: bridge: make ebt_snat ARP rewrite writable This commit adds a skb_ensure_writable() call in ebt_snat_tg() so that the ARP sender hardware address range is writable before skb_header_pointer() and skb_store_bits() are called. It is a backport of upstream commit 67ba971ae025. The code change looks correct. My only comment is on the backport notes in the commit message. > diff --git a/net/bridge/netfilter/ebt_snat.c b/net/bridge/netfilter/ebt_snat.c > index 7dfbcdfc30e5..c9e229af0366 100644 > --- a/net/bridge/netfilter/ebt_snat.c > +++ b/net/bridge/netfilter/ebt_snat.c [ ... ] > @@ -31,6 +31,9 @@ ebt_snat_tg(struct sk_buff *skb, const struct > xt_action_param *par) > const struct arphdr *ap; > struct arphdr _ah; > > + if (skb_ensure_writable(skb, sizeof(_ah) + ETH_ALEN)) > + return EBT_DROP; > + > ap = skb_header_pointer(skb, 0, sizeof(_ah), &_ah); > if (ap == NULL) > return EBT_DROP; [Severity: Low] This isn't a bug in the code, but the backport notes in the commit message don't match the tree this patch applies to. The notes say "the function here already has skb_make_writable(skb, 0)" and "the rest of the file still uses skb_make_writable()". They also say that 63137bc5882a and the v5.3 conversion c1a831167901 are not present in this tree. In the tree as it stands with this patch applied, ebt_snat_tg() already begins with: if (skb_ensure_writable(skb, 0)) return EBT_DROP; The sibling targets ebt_dnat_tg() and ebt_redirect_tg() also call skb_ensure_writable(), and I found no skb_make_writable() left in net/bridge/netfilter/. The patch only adds three lines, so the baseline already had the skb_ensure_writable(skb, 0) call. These notes look like they were written for the vz7 (3.10) version of the patch. The statements "applies unchanged" and "63137bc5882a from Fixes: is not here" may need to be corrected for this branch. The mention of RHEL7 carrying the same change may also not belong in the notes for this branch. -- Sashiko AI review ยท https://sashiko.vzint.dev/#/patchset/20261109-b4-prep-khorenko-vstor-154194-ebt_snat-arp-writable-v1-1-67df71125...@virtuozzo.com?part=1 _______________________________________________ Devel mailing list [email protected] https://lists.openvz.org/mailman/listinfo/devel
