Re: [PATCH] allow 0 protocol for bind() of packet sockets

[Laurence <laudney@xxxxxxxx>]

After reading the patch, I found some problems.
1. The patch doesn't allow "protocol == 0" either, even exit the
funtion at an earlier stage by shifting the place of
"if (protocol == 0)
  goto out_unlock;"
forward.

2. all the bulk changes to the codes of
if(dev) {
..
}
are simply an optimization. No content change at all!!

3. According to my understanding, I have made some modifications.
Hope that works.

First, maintain the following modification that's present in your patch:
@@ -920,7 +914,7 @@
                if (dev == NULL)
                        goto out;
        }
-       err = packet_do_bind(sk, dev, sll->sll_protocol ? : sk->num);
+       err = packet_do_bind(sk, dev, sll->sll_protocol);
        if (dev)
                dev_put(dev);
 

Then, the following is my packet_do_bind() funtion in net/packet/af_packet.c


static int packet_do_bind(struct sock *sk, struct net_device *dev, int protocol)
{
        /*
         *      Detach an existing hook if present.
         */

        lock_sock(sk);

        spin_lock(&sk->protinfo.af_packet->bind_lock);
        if (sk->protinfo.af_packet->running) {
                dev_remove_pack(&sk->protinfo.af_packet->prot_hook);
                __sock_put(sk);
                sk->protinfo.af_packet->running = 0;
        }


/* Think about the meaning of "err = packet_do_bind(sk, dev, sll->sll_protocol 
? : sk->num);"
 * If sll->sll_protocol==0, the parameter "protocol" in this funtion equals 
sk->num,
 * which means, sk->num is actually not modified. So, only sk->num = protocol 
when
 */ protocol != 0

        if (protocol != 0)
                sk->num = protocol;

        sk->protinfo.af_packet->prot_hook.type = protocol;
        sk->protinfo.af_packet->prot_hook.dev = dev;

        sk->protinfo.af_packet->ifindex = dev ? dev->ifindex : 0;

        if (dev) {
                if (dev->flags&IFF_UP) {
                        dev_add_pack(&sk->protinfo.af_packet->prot_hook);
                        sock_hold(sk);
                        sk->protinfo.af_packet->running = 1;
                } else {
                        sk->err = ENETDOWN;
                        if (!sk->dead)
                                sk->error_report(sk);
                }
        } else {
                dev_add_pack(&sk->protinfo.af_packet->prot_hook);
                sock_hold(sk);
                sk->protinfo.af_packet->running = 1;
        }

/* since protocol == 0, no hook should exist */
        if (protocol == 0)
                sk->protinfo.af_packet->running = 0;

out_unlock:
        spin_unlock(&sk->protinfo.af_packet->bind_lock);
        release_sock(sk);
        return 0;
}