> On 11 Jul 2018, at 05:45, Hannu Nyman <hannu.ny...@iki.fi> wrote: > > Stijn Segers kirjoitti 10.7.2018 klo 23:08: >> Refreshed patches. The bump from .53 to .54 introduced a minor change in >> net/netfilter/nf_tables_api.c [1] but I am unable to >> judge if this is a fluke or not, so I'd like a second pair of eyes on that. >> It's a single 'table[0]' being replaced by 'table': >> >> - if (filter && filter->table[0] && >> + if (filter && filter->table && >> >> I have updated the >> 335-v4.16-netfilter-nf_tables-add-single-table-list-for-all-fa.patch >> accordingly. >> > > Seems like a legitimate change due to upstream changes that are clearly > visible in your upstream diff link. > > Clicking your link and then looking at the file's commit log, I luckily > stumbled directly to the responsible commit (fix NULL pointer): > > https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux-stable.git/commit/net/netfilter/nf_tables_api.c?id=360cc79d9d299ce297b205508276285ceffc5fa8 > > Note also that our patch 335 removes the whole code block where that one line > changed in upstream. So, the change inside the removed code block would be > rather safe in any case. > > >> >> [1] >> https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux-stable.git/diff/net/netfilter/nf_tables_api.c?id=v4.14.54&id2=v4.14.53 >> > >> @@ -895,7 +895,7 @@ Signed-off-by: Pablo Neira Ayuso <pa...@netfilter.org> >> const struct nft_table *table; >> unsigned int idx = 0, s_idx = cb->args[0]; >> struct nft_obj_filter *filter = cb->data; >> -@@ -4576,38 +4562,37 @@ static int nf_tables_dump_obj(struct sk_ >> +@@ -4619,38 +4605,37 @@ static int nf_tables_dump_obj(struct sk_ >> rcu_read_lock(); >> cb->seq = net->nft.base_seq; >> @@ -914,7 +914,7 @@ Signed-off-by: Pablo Neira Ayuso <pa...@netfilter.org> >> - if (idx > s_idx) >> - memset(&cb->args[1], 0, >> - sizeof(cb->args) - >> sizeof(cb->args[0])); >> -- if (filter && filter->table[0] && >> +- if (filter && filter->table && >> - strcmp(filter->table, table->name)) >> - goto cont; >> - if (filter && >> @@ -960,7 +960,7 @@ Signed-off-by: Pablo Neira Ayuso <pa...@netfilter.org> >> } >> } >
Really not convinced I agree with the "patch 335 removes the whole code block
where that one line changed in upstream”. Did a refresh myself, several times,
and patch 335 is a right confusing pain in the backside. I think the block in
question should look like:
@@ -4619,38 +4605,37 @@ static int nf_tables_dump_obj(struct sk_
rcu_read_lock();
cb->seq = net->nft.base_seq;
- list_for_each_entry_rcu(afi, &net->nft.af_info, list) {
- if (family != NFPROTO_UNSPEC && family != afi->family)
+ list_for_each_entry_rcu(table, &net->nft.tables, list) {
+ if (family != NFPROTO_UNSPEC && family != table->afi->family)
continue;
- list_for_each_entry_rcu(table, &afi->tables, list) {
- list_for_each_entry_rcu(obj, &table->objects, list) {
- if (!nft_is_active(net, obj))
- goto cont;
- if (idx < s_idx)
- goto cont;
- if (idx > s_idx)
- memset(&cb->args[1], 0,
- sizeof(cb->args) -
sizeof(cb->args[0]));
- if (filter && filter->table &&
- strcmp(filter->table, table->name))
- goto cont;
- if (filter &&
- filter->type != NFT_OBJECT_UNSPEC &&
- obj->ops->type->type != filter->type)
- goto cont;
+ list_for_each_entry_rcu(obj, &table->objects, list) {
+ if (!nft_is_active(net, obj))
+ goto cont;
+ if (idx < s_idx)
+ goto cont;
+ if (idx > s_idx)
+ memset(&cb->args[1], 0,
+ sizeof(cb->args) - sizeof(cb->args[0]));
+ if (filter && filter->table &&
+ strcmp(filter->table, table->name))
+ goto cont;
+ if (filter &&
+ filter->type != NFT_OBJECT_UNSPEC &&
+ obj->ops->type->type != filter->type)
+ goto cont;
- if (nf_tables_fill_obj_info(skb, net,
NETLINK_CB(cb->skb).portid,
- cb->nlh->nlmsg_seq,
- NFT_MSG_NEWOBJ,
- NLM_F_MULTI |
NLM_F_APPEND,
- afi->family, table,
obj, reset) < 0)
- goto done;
+ if (nf_tables_fill_obj_info(skb, net,
NETLINK_CB(cb->skb).portid,
+ cb->nlh->nlmsg_seq,
+ NFT_MSG_NEWOBJ,
+ NLM_F_MULTI | NLM_F_APPEND,
+ table->afi->family, table,
+ obj, reset) < 0)
+ goto done;
- nl_dump_check_consistent(cb, nlmsg_hdr(skb));
-cont:
- idx++;
- }
+ nl_dump_check_consistent(cb, nlmsg_hdr(skb));
+ cont:
+ idx++;
}
}
done:
There are a couple of similar blocks, which have probably confused me anyway.
Overall this one patch in the refresh makes me distinctly uncomfortable.
>
> _______________________________________________
> openwrt-devel mailing list
> openwrt-devel@lists.openwrt.org
> https://lists.openwrt.org/mailman/listinfo/openwrt-devel
Cheers,
Kevin D-B
012C ACB2 28C6 C53E 9775 9123 B3A2 389B 9DE2 334A
signature.asc
Description: Message signed with OpenPGP
_______________________________________________ openwrt-devel mailing list openwrt-devel@lists.openwrt.org https://lists.openwrt.org/mailman/listinfo/openwrt-devel
