All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] net: memory leak in nr_rx_frame
@ 2023-11-10 17:36 Bragatheswaran Manickavel
  2023-11-10 21:31 ` Kuniyuki Iwashima
                   ` (2 more replies)
  0 siblings, 3 replies; 5+ messages in thread
From: Bragatheswaran Manickavel @ 2023-11-10 17:36 UTC (permalink / raw)
  To: ralf, davem, edumazet, kuba, pabeni
  Cc: Bragatheswaran Manickavel, linux-hams, netdev, linux-kernel,
	syzbot+0145ea560de205bc09f0

The condition (make = nr_make_new(sk)) == NULL suggests
that nr_make_new allocates memory and returns a pointer.
If this allocation fails (returns NULL), it indicates a
potential memory leak.

Added sock_put() for make which can potentially solve
this issue

Reported-by: syzbot+0145ea560de205bc09f0@syzkaller.appspotmail.com
Closes: https://syzkaller.appspot.com/bug?extid=0145ea560de205bc09f0
Signed-off-by: Bragatheswaran Manickavel <bragathemanick0908@gmail.com>
---
 net/netrom/af_netrom.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/net/netrom/af_netrom.c b/net/netrom/af_netrom.c
index 0eed00184adf..7d7cda4ae300 100644
--- a/net/netrom/af_netrom.c
+++ b/net/netrom/af_netrom.c
@@ -970,6 +970,8 @@ int nr_rx_frame(struct sk_buff *skb, struct net_device *dev)
 		nr_transmit_refusal(skb, 0);
 		if (sk)
 			sock_put(sk);
+		if (make)
+			sock_put(make);
 		return 0;
 	}
 
-- 
2.34.1


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

* Re: [PATCH] net: memory leak in nr_rx_frame
  2023-11-10 17:36 [PATCH] net: memory leak in nr_rx_frame Bragatheswaran Manickavel
@ 2023-11-10 21:31 ` Kuniyuki Iwashima
  2023-11-11  6:16   ` Bragatheswaran Manickavel
  2023-11-11  6:37 ` kernel test robot
  2023-11-13  0:56 ` Willem de Bruijn
  2 siblings, 1 reply; 5+ messages in thread
From: Kuniyuki Iwashima @ 2023-11-10 21:31 UTC (permalink / raw)
  To: bragathemanick0908
  Cc: davem, edumazet, kuba, linux-hams, linux-kernel, netdev, pabeni,
	ralf, syzbot+0145ea560de205bc09f0, kuniyu

From: Bragatheswaran Manickavel <bragathemanick0908@gmail.com>
Date: Fri, 10 Nov 2023 23:06:32 +0530
> The condition (make = nr_make_new(sk)) == NULL suggests
> that nr_make_new allocates memory and returns a pointer.
> If this allocation fails (returns NULL), it indicates a
> potential memory leak.

If make is NULL, nothing is allocated and leaked here, and
your code will never be executed as "if (make)" is always false.


> 
> Added sock_put() for make which can potentially solve
> this issue

Sounds like the patch is not tested with kmemleak.


> 
> Reported-by: syzbot+0145ea560de205bc09f0@syzkaller.appspotmail.com
> Closes: https://syzkaller.appspot.com/bug?extid=0145ea560de205bc09f0
> Signed-off-by: Bragatheswaran Manickavel <bragathemanick0908@gmail.com>
> ---
>  net/netrom/af_netrom.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/net/netrom/af_netrom.c b/net/netrom/af_netrom.c
> index 0eed00184adf..7d7cda4ae300 100644
> --- a/net/netrom/af_netrom.c
> +++ b/net/netrom/af_netrom.c
> @@ -970,6 +970,8 @@ int nr_rx_frame(struct sk_buff *skb, struct net_device *dev)
>  		nr_transmit_refusal(skb, 0);
>  		if (sk)
>  			sock_put(sk);
> +		if (make)
> +			sock_put(make);

Also, make could be uninitialised here if the first two
condition is true.

   if (sk == NULL || sk_acceptq_is_full(sk) ||


>  		return 0;
>  	}
>  
> -- 
> 2.34.1

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

* Re: [PATCH] net: memory leak in nr_rx_frame
  2023-11-10 21:31 ` Kuniyuki Iwashima
@ 2023-11-11  6:16   ` Bragatheswaran Manickavel
  0 siblings, 0 replies; 5+ messages in thread
From: Bragatheswaran Manickavel @ 2023-11-11  6:16 UTC (permalink / raw)
  To: Kuniyuki Iwashima
  Cc: davem, edumazet, kuba, linux-hams, linux-kernel, netdev, pabeni,
	ralf, syzbot+0145ea560de205bc09f0


On 11/11/23 03:01, Kuniyuki Iwashima wrote:
> From: Bragatheswaran Manickavel <bragathemanick0908@gmail.com>
> Date: Fri, 10 Nov 2023 23:06:32 +0530
>> The condition (make = nr_make_new(sk)) == NULL suggests
>> that nr_make_new allocates memory and returns a pointer.
>> If this allocation fails (returns NULL), it indicates a
>> potential memory leak.
> If make is NULL, nothing is allocated and leaked here, and
> your code will never be executed as "if (make)" is always false.
>
>
>> Added sock_put() for make which can potentially solve
>> this issue
> Sounds like the patch is not tested with kmemleak.

Yes, Had some issue with reproducer. So, haven't tested it

>
>
>> Reported-by: syzbot+0145ea560de205bc09f0@syzkaller.appspotmail.com
>> Closes: https://syzkaller.appspot.com/bug?extid=0145ea560de205bc09f0
>> Signed-off-by: Bragatheswaran Manickavel <bragathemanick0908@gmail.com>
>> ---
>>   net/netrom/af_netrom.c | 2 ++
>>   1 file changed, 2 insertions(+)
>>
>> diff --git a/net/netrom/af_netrom.c b/net/netrom/af_netrom.c
>> index 0eed00184adf..7d7cda4ae300 100644
>> --- a/net/netrom/af_netrom.c
>> +++ b/net/netrom/af_netrom.c
>> @@ -970,6 +970,8 @@ int nr_rx_frame(struct sk_buff *skb, struct net_device *dev)
>>   		nr_transmit_refusal(skb, 0);
>>   		if (sk)
>>   			sock_put(sk);
>> +		if (make)
>> +			sock_put(make);
> Also, make could be uninitialised here if the first two
> condition is true.
>
>     if (sk == NULL || sk_acceptq_is_full(sk) ||
>
>
>>   		return 0;
>>   	}
>>   
>> -- 
>> 2.34.1


Sorry for the inconvenience, let me check the code and get back properly.

Thanks,
Bragathe



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

* Re: [PATCH] net: memory leak in nr_rx_frame
  2023-11-10 17:36 [PATCH] net: memory leak in nr_rx_frame Bragatheswaran Manickavel
  2023-11-10 21:31 ` Kuniyuki Iwashima
@ 2023-11-11  6:37 ` kernel test robot
  2023-11-13  0:56 ` Willem de Bruijn
  2 siblings, 0 replies; 5+ messages in thread
From: kernel test robot @ 2023-11-11  6:37 UTC (permalink / raw)
  To: Bragatheswaran Manickavel, ralf, davem, edumazet, kuba, pabeni
  Cc: llvm, oe-kbuild-all, Bragatheswaran Manickavel, linux-hams,
	netdev, linux-kernel, syzbot+0145ea560de205bc09f0

Hi Bragatheswaran,

kernel test robot noticed the following build warnings:

[auto build test WARNING on net-next/main]
[also build test WARNING on net/main linus/master horms-ipvs/master v6.6 next-20231110]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Bragatheswaran-Manickavel/net-memory-leak-in-nr_rx_frame/20231111-013821
base:   net-next/main
patch link:    https://lore.kernel.org/r/20231110173632.2511-1-bragathemanick0908%40gmail.com
patch subject: [PATCH] net: memory leak in nr_rx_frame
config: powerpc64-allyesconfig (https://download.01.org/0day-ci/archive/20231111/202311111418.ojyVQuqO-lkp@intel.com/config)
compiler: clang version 17.0.0 (https://github.com/llvm/llvm-project.git 4a5ac14ee968ff0ad5d2cc1ffa0299048db4c88a)
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20231111/202311111418.ojyVQuqO-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202311111418.ojyVQuqO-lkp@intel.com/

All warnings (new ones prefixed by >>):

>> net/netrom/af_netrom.c:968:6: warning: variable 'make' is used uninitialized whenever '||' condition is true [-Wsometimes-uninitialized]
     968 |         if (sk == NULL || sk_acceptq_is_full(sk) ||
         |             ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   net/netrom/af_netrom.c:973:7: note: uninitialized use occurs here
     973 |                 if (make)
         |                     ^~~~
   net/netrom/af_netrom.c:968:6: note: remove the '||' if its condition is always false
     968 |         if (sk == NULL || sk_acceptq_is_full(sk) ||
         |             ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>> net/netrom/af_netrom.c:968:6: warning: variable 'make' is used uninitialized whenever '||' condition is true [-Wsometimes-uninitialized]
     968 |         if (sk == NULL || sk_acceptq_is_full(sk) ||
         |             ^~~~~~~~~~
   net/netrom/af_netrom.c:973:7: note: uninitialized use occurs here
     973 |                 if (make)
         |                     ^~~~
   net/netrom/af_netrom.c:968:6: note: remove the '||' if its condition is always false
     968 |         if (sk == NULL || sk_acceptq_is_full(sk) ||
         |             ^~~~~~~~~~~~~
   net/netrom/af_netrom.c:874:19: note: initialize the variable 'make' to silence this warning
     874 |         struct sock *make;
         |                          ^
         |                           = NULL
   2 warnings generated.


vim +968 net/netrom/af_netrom.c

^1da177e4c3f41 Linus Torvalds            2005-04-16   870  
^1da177e4c3f41 Linus Torvalds            2005-04-16   871  int nr_rx_frame(struct sk_buff *skb, struct net_device *dev)
^1da177e4c3f41 Linus Torvalds            2005-04-16   872  {
^1da177e4c3f41 Linus Torvalds            2005-04-16   873  	struct sock *sk;
^1da177e4c3f41 Linus Torvalds            2005-04-16   874  	struct sock *make;
^1da177e4c3f41 Linus Torvalds            2005-04-16   875  	struct nr_sock *nr_make;
^1da177e4c3f41 Linus Torvalds            2005-04-16   876  	ax25_address *src, *dest, *user;
^1da177e4c3f41 Linus Torvalds            2005-04-16   877  	unsigned short circuit_index, circuit_id;
^1da177e4c3f41 Linus Torvalds            2005-04-16   878  	unsigned short peer_circuit_index, peer_circuit_id;
^1da177e4c3f41 Linus Torvalds            2005-04-16   879  	unsigned short frametype, flags, window, timeout;
^1da177e4c3f41 Linus Torvalds            2005-04-16   880  	int ret;
^1da177e4c3f41 Linus Torvalds            2005-04-16   881  
c8c8218ec5af5d Cong Wang                 2019-06-27   882  	skb_orphan(skb);
^1da177e4c3f41 Linus Torvalds            2005-04-16   883  
^1da177e4c3f41 Linus Torvalds            2005-04-16   884  	/*
^1da177e4c3f41 Linus Torvalds            2005-04-16   885  	 *	skb->data points to the netrom frame start
^1da177e4c3f41 Linus Torvalds            2005-04-16   886  	 */
^1da177e4c3f41 Linus Torvalds            2005-04-16   887  
^1da177e4c3f41 Linus Torvalds            2005-04-16   888  	src  = (ax25_address *)(skb->data + 0);
^1da177e4c3f41 Linus Torvalds            2005-04-16   889  	dest = (ax25_address *)(skb->data + 7);
^1da177e4c3f41 Linus Torvalds            2005-04-16   890  
^1da177e4c3f41 Linus Torvalds            2005-04-16   891  	circuit_index      = skb->data[15];
^1da177e4c3f41 Linus Torvalds            2005-04-16   892  	circuit_id         = skb->data[16];
^1da177e4c3f41 Linus Torvalds            2005-04-16   893  	peer_circuit_index = skb->data[17];
^1da177e4c3f41 Linus Torvalds            2005-04-16   894  	peer_circuit_id    = skb->data[18];
^1da177e4c3f41 Linus Torvalds            2005-04-16   895  	frametype          = skb->data[19] & 0x0F;
^1da177e4c3f41 Linus Torvalds            2005-04-16   896  	flags              = skb->data[19] & 0xF0;
^1da177e4c3f41 Linus Torvalds            2005-04-16   897  
^1da177e4c3f41 Linus Torvalds            2005-04-16   898  	/*
^1da177e4c3f41 Linus Torvalds            2005-04-16   899  	 * Check for an incoming IP over NET/ROM frame.
^1da177e4c3f41 Linus Torvalds            2005-04-16   900  	 */
98a82febb63404 Ralf Baechle              2005-08-24   901  	if (frametype == NR_PROTOEXT &&
98a82febb63404 Ralf Baechle              2005-08-24   902  	    circuit_index == NR_PROTO_IP && circuit_id == NR_PROTO_IP) {
^1da177e4c3f41 Linus Torvalds            2005-04-16   903  		skb_pull(skb, NR_NETWORK_LEN + NR_TRANSPORT_LEN);
badff6d01a8589 Arnaldo Carvalho de Melo  2007-03-13   904  		skb_reset_transport_header(skb);
^1da177e4c3f41 Linus Torvalds            2005-04-16   905  
^1da177e4c3f41 Linus Torvalds            2005-04-16   906  		return nr_rx_ip(skb, dev);
^1da177e4c3f41 Linus Torvalds            2005-04-16   907  	}
^1da177e4c3f41 Linus Torvalds            2005-04-16   908  
^1da177e4c3f41 Linus Torvalds            2005-04-16   909  	/*
^1da177e4c3f41 Linus Torvalds            2005-04-16   910  	 * Find an existing socket connection, based on circuit ID, if it's
^1da177e4c3f41 Linus Torvalds            2005-04-16   911  	 * a Connect Request base it on their circuit ID.
^1da177e4c3f41 Linus Torvalds            2005-04-16   912  	 *
^1da177e4c3f41 Linus Torvalds            2005-04-16   913  	 * Circuit ID 0/0 is not valid but it could still be a "reset" for a
^1da177e4c3f41 Linus Torvalds            2005-04-16   914  	 * circuit that no longer exists at the other end ...
^1da177e4c3f41 Linus Torvalds            2005-04-16   915  	 */
^1da177e4c3f41 Linus Torvalds            2005-04-16   916  
^1da177e4c3f41 Linus Torvalds            2005-04-16   917  	sk = NULL;
^1da177e4c3f41 Linus Torvalds            2005-04-16   918  
^1da177e4c3f41 Linus Torvalds            2005-04-16   919  	if (circuit_index == 0 && circuit_id == 0) {
^1da177e4c3f41 Linus Torvalds            2005-04-16   920  		if (frametype == NR_CONNACK && flags == NR_CHOKE_FLAG)
^1da177e4c3f41 Linus Torvalds            2005-04-16   921  			sk = nr_find_peer(peer_circuit_index, peer_circuit_id, src);
^1da177e4c3f41 Linus Torvalds            2005-04-16   922  	} else {
^1da177e4c3f41 Linus Torvalds            2005-04-16   923  		if (frametype == NR_CONNREQ)
^1da177e4c3f41 Linus Torvalds            2005-04-16   924  			sk = nr_find_peer(circuit_index, circuit_id, src);
^1da177e4c3f41 Linus Torvalds            2005-04-16   925  		else
^1da177e4c3f41 Linus Torvalds            2005-04-16   926  			sk = nr_find_socket(circuit_index, circuit_id);
^1da177e4c3f41 Linus Torvalds            2005-04-16   927  	}
^1da177e4c3f41 Linus Torvalds            2005-04-16   928  
^1da177e4c3f41 Linus Torvalds            2005-04-16   929  	if (sk != NULL) {
7314f5480f3e37 Cong Wang                 2018-12-29   930  		bh_lock_sock(sk);
badff6d01a8589 Arnaldo Carvalho de Melo  2007-03-13   931  		skb_reset_transport_header(skb);
^1da177e4c3f41 Linus Torvalds            2005-04-16   932  
^1da177e4c3f41 Linus Torvalds            2005-04-16   933  		if (frametype == NR_CONNACK && skb->len == 22)
^1da177e4c3f41 Linus Torvalds            2005-04-16   934  			nr_sk(sk)->bpqext = 1;
^1da177e4c3f41 Linus Torvalds            2005-04-16   935  		else
^1da177e4c3f41 Linus Torvalds            2005-04-16   936  			nr_sk(sk)->bpqext = 0;
^1da177e4c3f41 Linus Torvalds            2005-04-16   937  
^1da177e4c3f41 Linus Torvalds            2005-04-16   938  		ret = nr_process_rx_frame(sk, skb);
^1da177e4c3f41 Linus Torvalds            2005-04-16   939  		bh_unlock_sock(sk);
7314f5480f3e37 Cong Wang                 2018-12-29   940  		sock_put(sk);
^1da177e4c3f41 Linus Torvalds            2005-04-16   941  		return ret;
^1da177e4c3f41 Linus Torvalds            2005-04-16   942  	}
^1da177e4c3f41 Linus Torvalds            2005-04-16   943  
^1da177e4c3f41 Linus Torvalds            2005-04-16   944  	/*
^1da177e4c3f41 Linus Torvalds            2005-04-16   945  	 * Now it should be a CONNREQ.
^1da177e4c3f41 Linus Torvalds            2005-04-16   946  	 */
^1da177e4c3f41 Linus Torvalds            2005-04-16   947  	if (frametype != NR_CONNREQ) {
^1da177e4c3f41 Linus Torvalds            2005-04-16   948  		/*
^1da177e4c3f41 Linus Torvalds            2005-04-16   949  		 * Here it would be nice to be able to send a reset but
e21ce8c7c013fb Ralf Baechle              2005-09-12   950  		 * NET/ROM doesn't have one.  We've tried to extend the protocol
e21ce8c7c013fb Ralf Baechle              2005-09-12   951  		 * by sending NR_CONNACK | NR_CHOKE_FLAGS replies but that
e21ce8c7c013fb Ralf Baechle              2005-09-12   952  		 * apparently kills BPQ boxes... :-(
e21ce8c7c013fb Ralf Baechle              2005-09-12   953  		 * So now we try to follow the established behaviour of
e21ce8c7c013fb Ralf Baechle              2005-09-12   954  		 * G8PZT's Xrouter which is sending packets with command type 7
e21ce8c7c013fb Ralf Baechle              2005-09-12   955  		 * as an extension of the protocol.
^1da177e4c3f41 Linus Torvalds            2005-04-16   956  		 */
e21ce8c7c013fb Ralf Baechle              2005-09-12   957  		if (sysctl_netrom_reset_circuit &&
e21ce8c7c013fb Ralf Baechle              2005-09-12   958  		    (frametype != NR_RESET || flags != 0))
e21ce8c7c013fb Ralf Baechle              2005-09-12   959  			nr_transmit_reset(skb, 1);
e21ce8c7c013fb Ralf Baechle              2005-09-12   960  
^1da177e4c3f41 Linus Torvalds            2005-04-16   961  		return 0;
^1da177e4c3f41 Linus Torvalds            2005-04-16   962  	}
^1da177e4c3f41 Linus Torvalds            2005-04-16   963  
^1da177e4c3f41 Linus Torvalds            2005-04-16   964  	sk = nr_find_listener(dest);
^1da177e4c3f41 Linus Torvalds            2005-04-16   965  
^1da177e4c3f41 Linus Torvalds            2005-04-16   966  	user = (ax25_address *)(skb->data + 21);
^1da177e4c3f41 Linus Torvalds            2005-04-16   967  
^1da177e4c3f41 Linus Torvalds            2005-04-16  @968  	if (sk == NULL || sk_acceptq_is_full(sk) ||
^1da177e4c3f41 Linus Torvalds            2005-04-16   969  	    (make = nr_make_new(sk)) == NULL) {
^1da177e4c3f41 Linus Torvalds            2005-04-16   970  		nr_transmit_refusal(skb, 0);
^1da177e4c3f41 Linus Torvalds            2005-04-16   971  		if (sk)
7314f5480f3e37 Cong Wang                 2018-12-29   972  			sock_put(sk);
d05be0015c62fa Bragatheswaran Manickavel 2023-11-10   973  		if (make)
d05be0015c62fa Bragatheswaran Manickavel 2023-11-10   974  			sock_put(make);
^1da177e4c3f41 Linus Torvalds            2005-04-16   975  		return 0;
^1da177e4c3f41 Linus Torvalds            2005-04-16   976  	}
^1da177e4c3f41 Linus Torvalds            2005-04-16   977  
7314f5480f3e37 Cong Wang                 2018-12-29   978  	bh_lock_sock(sk);
7314f5480f3e37 Cong Wang                 2018-12-29   979  
^1da177e4c3f41 Linus Torvalds            2005-04-16   980  	window = skb->data[20];
^1da177e4c3f41 Linus Torvalds            2005-04-16   981  
4638faac032756 Cong Wang                 2019-07-22   982  	sock_hold(make);
^1da177e4c3f41 Linus Torvalds            2005-04-16   983  	skb->sk             = make;
c8c8218ec5af5d Cong Wang                 2019-06-27   984  	skb->destructor     = sock_efree;
^1da177e4c3f41 Linus Torvalds            2005-04-16   985  	make->sk_state	    = TCP_ESTABLISHED;
^1da177e4c3f41 Linus Torvalds            2005-04-16   986  
^1da177e4c3f41 Linus Torvalds            2005-04-16   987  	/* Fill in his circuit details */
^1da177e4c3f41 Linus Torvalds            2005-04-16   988  	nr_make = nr_sk(make);
^1da177e4c3f41 Linus Torvalds            2005-04-16   989  	nr_make->source_addr = *dest;
^1da177e4c3f41 Linus Torvalds            2005-04-16   990  	nr_make->dest_addr   = *src;
^1da177e4c3f41 Linus Torvalds            2005-04-16   991  	nr_make->user_addr   = *user;
^1da177e4c3f41 Linus Torvalds            2005-04-16   992  
^1da177e4c3f41 Linus Torvalds            2005-04-16   993  	nr_make->your_index  = circuit_index;
^1da177e4c3f41 Linus Torvalds            2005-04-16   994  	nr_make->your_id     = circuit_id;
^1da177e4c3f41 Linus Torvalds            2005-04-16   995  
^1da177e4c3f41 Linus Torvalds            2005-04-16   996  	bh_unlock_sock(sk);
^1da177e4c3f41 Linus Torvalds            2005-04-16   997  	circuit = nr_find_next_circuit();
^1da177e4c3f41 Linus Torvalds            2005-04-16   998  	bh_lock_sock(sk);
^1da177e4c3f41 Linus Torvalds            2005-04-16   999  
^1da177e4c3f41 Linus Torvalds            2005-04-16  1000  	nr_make->my_index    = circuit / 256;
^1da177e4c3f41 Linus Torvalds            2005-04-16  1001  	nr_make->my_id       = circuit % 256;
^1da177e4c3f41 Linus Torvalds            2005-04-16  1002  
^1da177e4c3f41 Linus Torvalds            2005-04-16  1003  	circuit++;
^1da177e4c3f41 Linus Torvalds            2005-04-16  1004  
^1da177e4c3f41 Linus Torvalds            2005-04-16  1005  	/* Window negotiation */
^1da177e4c3f41 Linus Torvalds            2005-04-16  1006  	if (window < nr_make->window)
^1da177e4c3f41 Linus Torvalds            2005-04-16  1007  		nr_make->window = window;
^1da177e4c3f41 Linus Torvalds            2005-04-16  1008  
^1da177e4c3f41 Linus Torvalds            2005-04-16  1009  	/* L4 timeout negotiation */
^1da177e4c3f41 Linus Torvalds            2005-04-16  1010  	if (skb->len == 37) {
^1da177e4c3f41 Linus Torvalds            2005-04-16  1011  		timeout = skb->data[36] * 256 + skb->data[35];
^1da177e4c3f41 Linus Torvalds            2005-04-16  1012  		if (timeout * HZ < nr_make->t1)
^1da177e4c3f41 Linus Torvalds            2005-04-16  1013  			nr_make->t1 = timeout * HZ;
^1da177e4c3f41 Linus Torvalds            2005-04-16  1014  		nr_make->bpqext = 1;
^1da177e4c3f41 Linus Torvalds            2005-04-16  1015  	} else {
^1da177e4c3f41 Linus Torvalds            2005-04-16  1016  		nr_make->bpqext = 0;
^1da177e4c3f41 Linus Torvalds            2005-04-16  1017  	}
^1da177e4c3f41 Linus Torvalds            2005-04-16  1018  
^1da177e4c3f41 Linus Torvalds            2005-04-16  1019  	nr_write_internal(make, NR_CONNACK);
^1da177e4c3f41 Linus Torvalds            2005-04-16  1020  
^1da177e4c3f41 Linus Torvalds            2005-04-16  1021  	nr_make->condition = 0x00;
^1da177e4c3f41 Linus Torvalds            2005-04-16  1022  	nr_make->vs        = 0;
^1da177e4c3f41 Linus Torvalds            2005-04-16  1023  	nr_make->va        = 0;
^1da177e4c3f41 Linus Torvalds            2005-04-16  1024  	nr_make->vr        = 0;
^1da177e4c3f41 Linus Torvalds            2005-04-16  1025  	nr_make->vl        = 0;
^1da177e4c3f41 Linus Torvalds            2005-04-16  1026  	nr_make->state     = NR_STATE_3;
18601a7d30c834 Ralf Baechle              2006-07-03  1027  	sk_acceptq_added(sk);
^1da177e4c3f41 Linus Torvalds            2005-04-16  1028  	skb_queue_head(&sk->sk_receive_queue, skb);
^1da177e4c3f41 Linus Torvalds            2005-04-16  1029  
^1da177e4c3f41 Linus Torvalds            2005-04-16  1030  	if (!sock_flag(sk, SOCK_DEAD))
676d23690fb62b David S. Miller           2014-04-11  1031  		sk->sk_data_ready(sk);
^1da177e4c3f41 Linus Torvalds            2005-04-16  1032  
^1da177e4c3f41 Linus Torvalds            2005-04-16  1033  	bh_unlock_sock(sk);
7314f5480f3e37 Cong Wang                 2018-12-29  1034  	sock_put(sk);
5cc29e3bea7709 Ralf Baechle              2006-07-10  1035  
5cc29e3bea7709 Ralf Baechle              2006-07-10  1036  	nr_insert_socket(make);
5cc29e3bea7709 Ralf Baechle              2006-07-10  1037  
5cc29e3bea7709 Ralf Baechle              2006-07-10  1038  	nr_start_heartbeat(make);
5cc29e3bea7709 Ralf Baechle              2006-07-10  1039  	nr_start_idletimer(make);
5cc29e3bea7709 Ralf Baechle              2006-07-10  1040  
^1da177e4c3f41 Linus Torvalds            2005-04-16  1041  	return 1;
^1da177e4c3f41 Linus Torvalds            2005-04-16  1042  }
^1da177e4c3f41 Linus Torvalds            2005-04-16  1043  

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

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

* Re: [PATCH] net: memory leak in nr_rx_frame
  2023-11-10 17:36 [PATCH] net: memory leak in nr_rx_frame Bragatheswaran Manickavel
  2023-11-10 21:31 ` Kuniyuki Iwashima
  2023-11-11  6:37 ` kernel test robot
@ 2023-11-13  0:56 ` Willem de Bruijn
  2 siblings, 0 replies; 5+ messages in thread
From: Willem de Bruijn @ 2023-11-13  0:56 UTC (permalink / raw)
  To: Bragatheswaran Manickavel, ralf, davem, edumazet, kuba, pabeni
  Cc: Bragatheswaran Manickavel, linux-hams, netdev, linux-kernel,
	syzbot+0145ea560de205bc09f0

Bragatheswaran Manickavel wrote:
> The condition (make = nr_make_new(sk)) == NULL suggests
> that nr_make_new allocates memory and returns a pointer.
> If this allocation fails (returns NULL), it indicates a
> potential memory leak.
> 
> Added sock_put() for make which can potentially solve
> this issue
> 
> Reported-by: syzbot+0145ea560de205bc09f0@syzkaller.appspotmail.com
> Closes: https://syzkaller.appspot.com/bug?extid=0145ea560de205bc09f0
> Signed-off-by: Bragatheswaran Manickavel <bragathemanick0908@gmail.com>
> ---
>  net/netrom/af_netrom.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/net/netrom/af_netrom.c b/net/netrom/af_netrom.c
> index 0eed00184adf..7d7cda4ae300 100644
> --- a/net/netrom/af_netrom.c
> +++ b/net/netrom/af_netrom.c
> @@ -970,6 +970,8 @@ int nr_rx_frame(struct sk_buff *skb, struct net_device *dev)
>  		nr_transmit_refusal(skb, 0);
>  		if (sk)
>  			sock_put(sk);
> +		if (make)
> +			sock_put(make);
>  		return 0;
>  	}

If the branch is entered because (make = nr_make_new(sk)) == NULL then
make is.. NULL. If the branch is entered for one if the earlier two
conditions failing, then make is undefined.

The syzbot report is that make is allocated here and eventually
nothing points to this still allocated memory. It does not report
where the last reference was lost. Not inside this branch.

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

end of thread, other threads:[~2023-11-13  0:57 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-11-10 17:36 [PATCH] net: memory leak in nr_rx_frame Bragatheswaran Manickavel
2023-11-10 21:31 ` Kuniyuki Iwashima
2023-11-11  6:16   ` Bragatheswaran Manickavel
2023-11-11  6:37 ` kernel test robot
2023-11-13  0:56 ` Willem de Bruijn

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.