[OpenWrt-Devel] [PATCH 2/2] [RFC] kernel: bump 4.14 to 4.14.54 for 18.06

Kevin Darbyshire-Bryant kevin at darbyshire-bryant.me.uk
Wed Jul 11 04:11:46 EDT 2018



> On 11 Jul 2018, at 05:45, Hannu Nyman <hannu.nyman at 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 <pablo at 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 <pablo at 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 <pablo at 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 at 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

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: Message signed with OpenPGP
URL: <http://lists.infradead.org/pipermail/openwrt-devel/attachments/20180711/afc4a9b6/attachment.sig>
-------------- next part --------------
_______________________________________________
openwrt-devel mailing list
openwrt-devel at lists.openwrt.org
https://lists.openwrt.org/mailman/listinfo/openwrt-devel


More information about the openwrt-devel mailing list