netdev - Re: [GENETLINK]: Question: global lock (genl

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Date:	Fri, 17 Aug 2007 10:38:59 +0200
From:	Richard MUSIL <richard.musil@...com>
To:	Thomas Graf <tgraf@...g.ch>
Cc:	Patrick McHardy <kaber@...sh.net>, netdev@...r.kernel.org
Subject: Re: [GENETLINK]: Question: global lock (genl_mutex) possible refinement?

Thomas Graf wrote:
>> @@ -150,9 +176,9 @@ int genl_register_ops(struct genl_family *family, struct genl_ops *ops)
>>  	if (ops->policy)
>>  		ops->flags |= GENL_CMD_CAP_HASPOL;
>>  
>> -	genl_lock();
>> +	genl_fam_lock(family);
>>  	list_add_tail(&ops->ops_list, &family->ops_list);
>> -	genl_unlock();
>> +	genl_fam_unlock(family);
> 
> For registering operations, it is sufficient to just acquire the
> family lock, the family itself can't disappear while holding it.

I agree.

>> @@ -303,38 +332,57 @@ static int genl_rcv_msg(struct sk_buff *skb, struct nlmsghdr *nlh)
>>  	struct genlmsghdr *hdr = nlmsg_data(nlh);
>>  	int hdrlen, err;
>>  
>> +	genl_fam_lock(NULL);
>>  	family = genl_family_find_byid(nlh->nlmsg_type);
>> -	if (family == NULL)
>> +	if (family == NULL) {
>> +		genl_fam_unlock(NULL);
>>  		return -ENOENT;
>> +	}
>> +
>> +	/* get particular family lock, but release global family lock
>> +	 * so registering operations for other families are possible */
>> +	genl_onefam_lock(family);
>> +	genl_fam_unlock(NULL);
> 
> I don't like having two locks for something as trivial as this.
> Basically the only reason the global lock is required here is to
> protect from family removal which can be avoided otherwise by
> using RCU list operations.
> 
> Therefore, I'd propose the following lock semantics:
> Use own global mutex to protect writing to the family list, make
> reading side lockless using rcu for use when looking up family
> upon mesage processing. Use a family lock to protect writing to
> operations list and serialize messae processing with unregister
> operations.

I was not aware of RCU lists, but after looking at them, I consider your
proposal to be better. I guess, you would rather write the patch
yourself, so I will wait.

Thanks for help,
Richard
-
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html