Re: [PATCH 1/2] genl: Fix genl dumpit() locking.

[Johannes Berg <johannes@sipsolutions.net>]

> @@ -572,15 +594,29 @@ static int genl_family_rcv_msg(struct
> genl_family *family,
>                 return -EPERM;
>  
>         if ((nlh->nlmsg_flags & NLM_F_DUMP) == NLM_F_DUMP) {
> -               struct netlink_dump_control c = {
> -                       .dump = ops->dumpit,
> -                       .done = ops->done,
> -               };
> +               struct netlink_dump_control c;
> +               int rc;
>  
>                 if (ops->dumpit == NULL)
>                         return -EOPNOTSUPP;
>  
> -               return netlink_dump_start(net->genl_sock, skb, nlh,
> &c);
> +               memset(&c, 0, sizeof(c));
> +               if (!family->parallel_ops) {
> +                       genl_unlock();
> +                       c.data = ops;
> +                       c.dump = genl_lock_dumpit;
> +                       if (ops->done)
> +                               c.done = genl_lock_done;
> +               } else {
> +                       c.dump = ops->dumpit;
> +                       c.done = ops->done;
> +               }
> +
> +               rc = netlink_dump_start(net->genl_sock, skb, nlh, &c);
> +               if (!family->parallel_ops)
> +                       genl_lock();
> +               return rc;
> +

I think this piece would be easier to read if you call
netlink_dump_start() separately in the two branches. If you also move
the "c" variable into the branches then you can keep initializing it
with an explicit initializer which would also be more readable IMHO.

Either way, this seems fine. I still think that not assigning cb_mutex
wasn't the smartest idea, but I'll reply over in the other thread.

johannes