ACK: [PATCH][TRUSTY][SRU] UBUNTU: SAUCE: (no-up) net/packet: fix erroneous dev_add_pack usage in fanout

[Kleber Souza]

On 10/27/18 03:00, Jay Vosburgh wrote:
> BugLink: http://bugs.launchpad.net/bugs/1800254
> 
> Due to changes added as part of c108ac876c02 ("packet: hold bind lock when
> rebinding to fanout hook"), it is possible for fanout_add to add a
> packet_type handler via dev_add_pack and then kfree the memory backing the
> packet_type.  This corrupts the ptype_all list, causing the system to
> panic when network packet processing next traverses ptype_all.  The
> erroneous path is taken when a PACKET_FANOUT setsockopt is performed on a
> packet socket that is bound to an interface that is administratively down.
> 
> This is not due to any flaw of c108ac876c02, but rather than the packet
> socket code base differs subtly in 3.13 as compared to 4.4.
> 
> The remedy for this is to include additional changes in the management
> of the dev_add_pack calls from 4.4.  This moves the dev_add_pack and
> dev_remove_pack calls from fanout_add and _release into __fanout_link
> and _unlink.
> 
> These changes originate in 2bd624b4611f ("packet: Do not call
> fanout_release from atomic contexts").  We do not include that patch in
> its entirety as it has other dependencies, and this is the minimal
> change set to resolve the issue.
> 
> Signed-off-by: Jay Vosburgh <jay.vosburgh at canonical.com>

Acked-by: Kleber Sacilotto de Souza <kleber.souza at canonical.com>


Thanks for the additional explanation on the other email!

> 
> ---
>  net/packet/af_packet.c | 6 ++++--
>  1 file changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/net/packet/af_packet.c b/net/packet/af_packet.c
> index c0230c7458df..fa02443df232 100644
> --- a/net/packet/af_packet.c
> +++ b/net/packet/af_packet.c
> @@ -1267,6 +1267,8 @@ static void __fanout_link(struct sock *sk, struct packet_sock *po)
>  	f->arr[f->num_members] = sk;
>  	smp_wmb();
>  	f->num_members++;
> +	if (f->num_members == 1)
> +		dev_add_pack(&f->prot_hook);
>  	spin_unlock(&f->lock);
>  }
>  
> @@ -1283,6 +1285,8 @@ static void __fanout_unlink(struct sock *sk, struct packet_sock *po)
>  	BUG_ON(i >= f->num_members);
>  	f->arr[i] = f->arr[f->num_members - 1];
>  	f->num_members--;
> +	if (f->num_members == 0)
> +		__dev_remove_pack(&f->prot_hook);
>  	spin_unlock(&f->lock);
>  }
>  
> @@ -1350,7 +1354,6 @@ static int fanout_add(struct sock *sk, u16 id, u16 type_flags)
>  		match->prot_hook.func = packet_rcv_fanout;
>  		match->prot_hook.af_packet_priv = match;
>  		match->prot_hook.id_match = match_fanout_group;
> -		dev_add_pack(&match->prot_hook);
>  		list_add(&match->list, &fanout_list);
>  	}
>  	err = -EINVAL;
> @@ -1393,7 +1396,6 @@ static void fanout_release(struct sock *sk)
>  
>  		if (atomic_dec_and_test(&f->sk_ref)) {
>  			list_del(&f->list);
> -			dev_remove_pack(&f->prot_hook);
>  			kfree(f);
>  		}
>  	}
>