linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] netfilter: nfnetlink: check 'skb->dev' pointer in nfulnl_log_packet()
@ 2022-12-02  8:33 Li Qiong
  2022-12-02 10:20 ` Dan Carpenter
  2022-12-05 14:28 ` Pablo Neira Ayuso
  0 siblings, 2 replies; 3+ messages in thread
From: Li Qiong @ 2022-12-02  8:33 UTC (permalink / raw)
  To: Pablo Neira Ayuso, Jozsef Kadlecsik, Florian Westphal,
	David S . Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni
  Cc: netdev, linux-kernel, netfilter-devel, kernel-janitors, coreteam,
	Yu Zhe, Li Qiong

The 'skb->dev' may be NULL, it should be better to check it.

Signed-off-by: Li Qiong <liqiong@nfschina.com>
---
 net/netfilter/nfnetlink_log.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/net/netfilter/nfnetlink_log.c b/net/netfilter/nfnetlink_log.c
index d97eb280cb2e..74ac9fa40137 100644
--- a/net/netfilter/nfnetlink_log.c
+++ b/net/netfilter/nfnetlink_log.c
@@ -572,7 +572,7 @@ __build_packet_message(struct nfnl_log_net *log,
 		}
 	}
 
-	if (indev && skb_mac_header_was_set(skb)) {
+	if (indev && skb->dev && skb_mac_header_was_set(skb)) {
 		if (nla_put_be16(inst->skb, NFULA_HWTYPE, htons(skb->dev->type)) ||
 		    nla_put_be16(inst->skb, NFULA_HWLEN,
 				 htons(skb->dev->hard_header_len)))
@@ -724,7 +724,7 @@ nfulnl_log_packet(struct net *net,
 		+ nla_total_size(sizeof(struct nfulnl_msg_packet_timestamp))
 		+ nla_total_size(sizeof(struct nfgenmsg));	/* NLMSG_DONE */
 
-	if (in && skb_mac_header_was_set(skb)) {
+	if (in && skb->dev && skb_mac_header_was_set(skb)) {
 		size += nla_total_size(skb->dev->hard_header_len)
 			+ nla_total_size(sizeof(u_int16_t))	/* hwtype */
 			+ nla_total_size(sizeof(u_int16_t));	/* hwlen */
-- 
2.11.0


^ permalink raw reply related	[flat|nested] 3+ messages in thread

* Re: [PATCH] netfilter: nfnetlink: check 'skb->dev' pointer in nfulnl_log_packet()
  2022-12-02  8:33 [PATCH] netfilter: nfnetlink: check 'skb->dev' pointer in nfulnl_log_packet() Li Qiong
@ 2022-12-02 10:20 ` Dan Carpenter
  2022-12-05 14:28 ` Pablo Neira Ayuso
  1 sibling, 0 replies; 3+ messages in thread
From: Dan Carpenter @ 2022-12-02 10:20 UTC (permalink / raw)
  To: Li Qiong
  Cc: Pablo Neira Ayuso, Jozsef Kadlecsik, Florian Westphal,
	David S . Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
	netdev, linux-kernel, netfilter-devel, kernel-janitors, coreteam,
	Yu Zhe

On Fri, Dec 02, 2022 at 04:33:04PM +0800, Li Qiong wrote:
> The 'skb->dev' may be NULL, it should be better to check it.
> 
> Signed-off-by: Li Qiong <liqiong@nfschina.com>
> ---

This needs a Fixes tag as well.

Here my records show that I looked at this in 2018 but I probably
ignored it based on that the code was introduced in 2012.  When warnings
are really ancient sometimes I just assume they must be false positives
or someone would have hit it in testing.

Also for really ancient bugs, it's hard to contact the original author
or they have forgotten the details of the code.

regards,
dan carpenter


^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH] netfilter: nfnetlink: check 'skb->dev' pointer in nfulnl_log_packet()
  2022-12-02  8:33 [PATCH] netfilter: nfnetlink: check 'skb->dev' pointer in nfulnl_log_packet() Li Qiong
  2022-12-02 10:20 ` Dan Carpenter
@ 2022-12-05 14:28 ` Pablo Neira Ayuso
  1 sibling, 0 replies; 3+ messages in thread
From: Pablo Neira Ayuso @ 2022-12-05 14:28 UTC (permalink / raw)
  To: Li Qiong
  Cc: Jozsef Kadlecsik, Florian Westphal, David S . Miller,
	Eric Dumazet, Jakub Kicinski, Paolo Abeni, netdev, linux-kernel,
	netfilter-devel, kernel-janitors, coreteam, Yu Zhe

On Fri, Dec 02, 2022 at 04:33:04PM +0800, Li Qiong wrote:
> The 'skb->dev' may be NULL, it should be better to check it.
> 
> Signed-off-by: Li Qiong <liqiong@nfschina.com>
> ---
>  net/netfilter/nfnetlink_log.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/net/netfilter/nfnetlink_log.c b/net/netfilter/nfnetlink_log.c
> index d97eb280cb2e..74ac9fa40137 100644
> --- a/net/netfilter/nfnetlink_log.c
> +++ b/net/netfilter/nfnetlink_log.c
> @@ -572,7 +572,7 @@ __build_packet_message(struct nfnl_log_net *log,
>  		}
>  	}
>  
> -	if (indev && skb_mac_header_was_set(skb)) {
> +	if (indev && skb->dev && skb_mac_header_was_set(skb)) {
>  		if (nla_put_be16(inst->skb, NFULA_HWTYPE, htons(skb->dev->type)) ||
>  		    nla_put_be16(inst->skb, NFULA_HWLEN,
>  				 htons(skb->dev->hard_header_len)))
> @@ -724,7 +724,7 @@ nfulnl_log_packet(struct net *net,
>  		+ nla_total_size(sizeof(struct nfulnl_msg_packet_timestamp))
>  		+ nla_total_size(sizeof(struct nfgenmsg));	/* NLMSG_DONE */
>  
> -	if (in && skb_mac_header_was_set(skb)) {
> +	if (in && skb->dev && skb_mac_header_was_set(skb)) {
>  		size += nla_total_size(skb->dev->hard_header_len)
>  			+ nla_total_size(sizeof(u_int16_t))	/* hwtype */
>  			+ nla_total_size(sizeof(u_int16_t));	/* hwlen */

skb->dev is always guaranteed to be set in this path.

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2022-12-05 14:28 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-12-02  8:33 [PATCH] netfilter: nfnetlink: check 'skb->dev' pointer in nfulnl_log_packet() Li Qiong
2022-12-02 10:20 ` Dan Carpenter
2022-12-05 14:28 ` Pablo Neira Ayuso

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).