Re: [PATCH v2 2/4] packet: Add fanout support.

[Eric Dumazet <eric.dumazet@gmail.com>]

Le mardi 05 juillet 2011 à 02:04 -0700, David Miller a écrit :


> +static int fanout_add(struct sock *sk, u16 id, u8 type)
> +{
> +	struct packet_sock *po = pkt_sk(sk);
> +	struct packet_fanout *f, *match;
> +	int err;
> +
> +	switch (type) {
> +	case PACKET_FANOUT_HASH:
> +	case PACKET_FANOUT_LB:
> +		break;
> +	default:
> +		return -EINVAL;
> +	}
> +
> +	if (!po->running)
> +		return -EINVAL;
> +
> +	if (po->fanout)
> +		return -EALREADY;
> +
> +	mutex_lock(&fanout_mutex);
> +	match = NULL;
> +	list_for_each_entry(f, &fanout_list, list) {
> +		if (f->id == id &&
> +		    read_pnet(&f->net) == sock_net(sk)) {
> +			match = f;
> +			break;
> +		}
> +	}
> +	if (!match) {
> +		match = kzalloc(sizeof(*match), GFP_KERNEL);
> +		if (match) {
> +			write_pnet(&match->net, sock_net(sk));
> +			match->id = id;
> +			match->type = type;
> +			atomic_set(&match->rr_cur, 0);
> +			INIT_LIST_HEAD(&match->list);
> +			spin_lock_init(&match->lock);
> +			atomic_set(&match->sk_ref, 0);
> +			match->prot_hook.type = po->prot_hook.type;
> +			match->prot_hook.dev = po->prot_hook.dev;
> +			switch (type) {
> +			case PACKET_FANOUT_HASH:
> +				match->prot_hook.func = packet_rcv_fanout_hash;
> +				break;
> +			case PACKET_FANOUT_LB:
> +				match->prot_hook.func = packet_rcv_fanout_lb;
> +				break;
> +			}
> +			match->prot_hook.af_packet_priv = match;
> +			dev_add_pack(&match->prot_hook);

There is a small window where __fanout_link(sk, po) is not yet called,
but packets can be received on other cpus since dev_add_pack() was
called.

There is no divide by 0, but fanout_demux_hash() can return a NULL sk,
thus we can crash...

> +			list_add(&match->list, &fanout_list);
> +		}
> +	}
> +	err = -ENOMEM;
> +	if (match) {
> +		err = -EINVAL;
> +		if (match->type == type &&
> +		    match->prot_hook.type == po->prot_hook.type &&
> +		    match->prot_hook.dev == po->prot_hook.dev) {
> +			err = -ENOSPC;
> +			if (atomic_read(&match->sk_ref) < PACKET_FANOUT_MAX) {
> +				__dev_remove_pack(&po->prot_hook);
> +				po->fanout = match;
> +				atomic_inc(&match->sk_ref);
> +				__fanout_link(sk, po);
> +				err = 0;
> +			}
> +		}
> +	}
> +	mutex_unlock(&fanout_mutex);
> +	return err;
> +}
> +