[ammarfaizi2-block:dhowells/linux-fs/rxrpc-ringless-5 19/77] net/rxrpc/input.c:519 rxrpc_input_data() warn: passing freed memory 'skb'

[ammarfaizi2-block:dhowells/linux-fs/rxrpc-ringless-5 19/77] net/rxrpc/input.c:519 rxrpc_input_data() warn: passing freed memory 'skb'

[Dan Carpenter]

tree:   https://github.com/ammarfaizi2/linux-block dhowells/linux-fs/rxrpc-ringless-5
head:   30d95efe06e18bd55691902bb4ec873e4b21a754
commit: dad511288b61094b347de3baa13077e648a40dec [19/77] rxrpc: Clone received jumbo subpackets and queue separately
config: openrisc-randconfig-m031-20221106
compiler: or1k-linux-gcc (GCC) 12.1.0

If you fix the issue, kindly add following tag where applicable
| Reported-by: kernel test robot <lkp@intel.com>
| Reported-by: Dan Carpenter <error27@gmail.com>

New smatch warnings:
net/rxrpc/input.c:519 rxrpc_input_data() warn: passing freed memory 'skb'

Old smatch warnings:
net/rxrpc/input.c:1269 rxrpc_input_packet() warn: passing freed memory 'skb'

vim +/skb +519 net/rxrpc/input.c

dad511288b6109 net/rxrpc/input.c    David Howells 2022-10-07  494  static void rxrpc_input_data(struct rxrpc_call *call, struct sk_buff *skb)
dad511288b6109 net/rxrpc/input.c    David Howells 2022-10-07  495  {
dad511288b6109 net/rxrpc/input.c    David Howells 2022-10-07  496  	struct rxrpc_skb_priv *sp = rxrpc_skb(skb);
dad511288b6109 net/rxrpc/input.c    David Howells 2022-10-07  497  	enum rxrpc_call_state state;
dad511288b6109 net/rxrpc/input.c    David Howells 2022-10-07  498  	rxrpc_serial_t serial = sp->hdr.serial;
dad511288b6109 net/rxrpc/input.c    David Howells 2022-10-07  499  	rxrpc_seq_t seq0 = sp->hdr.seq;
dad511288b6109 net/rxrpc/input.c    David Howells 2022-10-07  500  
dad511288b6109 net/rxrpc/input.c    David Howells 2022-10-07  501  	_enter("{%u,%u},{%u,%u}",
dad511288b6109 net/rxrpc/input.c    David Howells 2022-10-07  502  	       call->rx_hard_ack, call->rx_top, skb->len, seq0);
dad511288b6109 net/rxrpc/input.c    David Howells 2022-10-07  503  
dad511288b6109 net/rxrpc/input.c    David Howells 2022-10-07  504  	_proto("Rx DATA %%%u { #%u f=%02x }",
dad511288b6109 net/rxrpc/input.c    David Howells 2022-10-07  505  	       sp->hdr.serial, seq0, sp->hdr.flags);
dad511288b6109 net/rxrpc/input.c    David Howells 2022-10-07  506  
dad511288b6109 net/rxrpc/input.c    David Howells 2022-10-07  507  	state = READ_ONCE(call->state);
dad511288b6109 net/rxrpc/input.c    David Howells 2022-10-07  508  	if (state >= RXRPC_CALL_COMPLETE) {
dad511288b6109 net/rxrpc/input.c    David Howells 2022-10-07  509  		rxrpc_free_skb(skb, rxrpc_skb_freed);
dad511288b6109 net/rxrpc/input.c    David Howells 2022-10-07  510  		goto out;
dad511288b6109 net/rxrpc/input.c    David Howells 2022-10-07  511  	}
dad511288b6109 net/rxrpc/input.c    David Howells 2022-10-07  512  
dad511288b6109 net/rxrpc/input.c    David Howells 2022-10-07  513  	/* Unshare the packet so that it can be modified for in-place
dad511288b6109 net/rxrpc/input.c    David Howells 2022-10-07  514  	 * decryption.
dad511288b6109 net/rxrpc/input.c    David Howells 2022-10-07  515  	 */
dad511288b6109 net/rxrpc/input.c    David Howells 2022-10-07  516  	if (sp->hdr.securityIndex != 0) {
dad511288b6109 net/rxrpc/input.c    David Howells 2022-10-07  517  		struct sk_buff *nskb = skb_unshare(skb, GFP_NOFS);
dad511288b6109 net/rxrpc/input.c    David Howells 2022-10-07  518  		if (!nskb) {
dad511288b6109 net/rxrpc/input.c    David Howells 2022-10-07 @519  			rxrpc_eaten_skb(skb, rxrpc_skb_unshared_nomem);

We can't use "skb" after skb_unshare().  It means we dropped our
reference to the skb.  The other reference holder probably holds a
reference so it will probably work, but it could also race and lead to
a use after free.

This only affects tracing code and not regular runtime but it's still a
bug.

dad511288b6109 net/rxrpc/input.c    David Howells 2022-10-07  520  			return;
dad511288b6109 net/rxrpc/input.c    David Howells 2022-10-07  521  		}
dad511288b6109 net/rxrpc/input.c    David Howells 2022-10-07  522  
dad511288b6109 net/rxrpc/input.c    David Howells 2022-10-07  523  		if (nskb != skb) {
dad511288b6109 net/rxrpc/input.c    David Howells 2022-10-07  524  			rxrpc_eaten_skb(skb, rxrpc_skb_received);
dad511288b6109 net/rxrpc/input.c    David Howells 2022-10-07  525  			skb = nskb;
dad511288b6109 net/rxrpc/input.c    David Howells 2022-10-07  526  			rxrpc_new_skb(skb, rxrpc_skb_unshared);
dad511288b6109 net/rxrpc/input.c    David Howells 2022-10-07  527  			sp = rxrpc_skb(skb);
dad511288b6109 net/rxrpc/input.c    David Howells 2022-10-07  528  		}
dad511288b6109 net/rxrpc/input.c    David Howells 2022-10-07  529  	}
dad511288b6109 net/rxrpc/input.c    David Howells 2022-10-07  530  
dad511288b6109 net/rxrpc/input.c    David Howells 2022-10-07  531  	if (state == RXRPC_CALL_SERVER_RECV_REQUEST) {

-- 
0-DAY CI Kernel Test Service
https://01.org/lkp

Re: [ammarfaizi2-block:dhowells/linux-fs/rxrpc-ringless-5 19/77] net/rxrpc/input.c:519 rxrpc_input_data() warn: passing freed memory 'skb'

[David Howells]

Dan Carpenter <error27@gmail.com> wrote:

> We can't use "skb" after skb_unshare().  It means we dropped our
> reference to the skb.  The other reference holder probably holds a
> reference so it will probably work, but it could also race and lead to
> a use after free.
> 
> This only affects tracing code and not regular runtime but it's still a
> bug.

Nothing in rxrpc_eaten_skb() actually dereferences the skbuff pointer.  The
tracepoint merely logs the address.

David