netdev - Re: [PATCH] netlink: allocate group bitmaps dynamically

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] [thread-next>] [day] [month] [year] [list]

Message-ID: <468A3BA5.9050909@trash.net>
Date:	Tue, 03 Jul 2007 14:05:57 +0200
From:	Patrick McHardy <kaber@...sh.net>
To:	Johannes Berg <johannes@...solutions.net>
CC:	hadi@...erus.ca, Zhang Rui <rui.zhang@...el.com>,
	netdev@...r.kernel.org,
	"linux-acpi@...r" <linux-acpi@...r.kernel.org>, lenb@...nel.org,
	Thomas Graf <tgraf@...g.ch>
Subject: Re: [PATCH] netlink: allocate group bitmaps dynamically

Johannes Berg wrote:
> Allow changing the number of groups for a netlink family
> after it has been created.
> 
> Signed-off-by: Johannes Berg <johannes@...solutions.net>
> 
> ---
>  include/linux/netlink.h  |    1 
>  net/netlink/af_netlink.c |   61 +++++++++++++++++++++++++++++++++++------------
>  2 files changed, 47 insertions(+), 15 deletions(-)
> 
> --- wireless-dev.orig/net/netlink/af_netlink.c	2007-07-03 00:10:31.617889695 +0200
> +++ wireless-dev/net/netlink/af_netlink.c	2007-07-03 00:31:30.267889695 +0200
> @@ -316,8 +316,11 @@ netlink_update_listeners(struct sock *sk
>  
>  	for (i = 0; i < NLGRPSZ(tbl->groups)/sizeof(unsigned long); i++) {
>  		mask = 0;
> -		sk_for_each_bound(sk, node, &tbl->mc_list)
> -			mask |= nlk_sk(sk)->groups[i];
> +		sk_for_each_bound(sk, node, &tbl->mc_list) {
> +			if (nlk_sk(sk)->ngroups >=
> +			    (i + 1) * sizeof(unsigned long))


This condition implies that a socket can bind to a non-existant
group, which shouldn't be possible.

> +				mask |= nlk_sk(sk)->groups[i];
> +		}
>  		tbl->listeners[i] = mask;
>  	}
>  	/* this function is only called with the netlink table "grabbed", which
> @@ -555,10 +558,11 @@ netlink_update_subscriptions(struct sock
>  	nlk->subscriptions = subscriptions;
>  }
>  
> -static int netlink_alloc_groups(struct sock *sk)
> +static int netlink_realloc_groups(struct sock *sk)
>  {
>  	struct netlink_sock *nlk = nlk_sk(sk);
>  	unsigned int groups;
> +	unsigned long *new_groups;
>  	int err = 0;
>  
>  	netlink_lock_table();
> @@ -570,9 +574,15 @@ static int netlink_alloc_groups(struct s
>  	if (err)
>  		return err;
>  
> -	nlk->groups = kzalloc(NLGRPSZ(groups), GFP_KERNEL);
> -	if (nlk->groups == NULL)
> +	if (nlk->ngroups >= groups)
> +		return 0;
> +
> +	new_groups = krealloc(nlk->groups, NLGRPSZ(groups), GFP_KERNEL);
> +	if (new_groups == NULL)
>  		return -ENOMEM;
> +	memset((char*)new_groups + NLGRPSZ(nlk->ngroups), 0,
> +	       NLGRPSZ(groups) - NLGRPSZ(nlk->ngroups));
> +	nlk->groups = new_groups;


This should probably happen with the table grabbed to avoid races
with concurrent broadcasts.



> @@ -1332,6 +1338,31 @@ out_sock_release:
>  	return NULL;
>  }
>  
> +int netlink_change_ngroups(int unit, unsigned int groups)
> +{
> +	unsigned long *listeners;
> +	int err = 0;
> +
> +	netlink_table_grab();


Unfortunately that doesn't prevent races with netlink_has_listeners
since its lockless (and should really stay that way).


> +	if (NLGRPSZ(nl_table[unit].groups) < NLGRPSZ(groups)) {
> +		listeners = krealloc(nl_table[unit].listeners,
> +				     NLGRPSZ(groups), GFP_KERNEL);
> +		if (!listeners) {
> +			err = -ENOMEM;
> +			goto out_ungrab;
> +		}
> +		memset((char*) listeners + NLGRPSZ(nl_table[unit].groups),
> +		       0, NLGRPSZ(groups) - NLGRPSZ(nl_table[unit].groups));
> +		nl_table[unit].listeners = listeners;
> +	}
> +	nl_table[unit].groups = groups;
> +
> + out_ungrab:
> +	netlink_table_ungrab();
> +	return err;
> +}
> +EXPORT_SYMBOL(netlink_change_ngroups);
> +
>  void netlink_set_nonroot(int protocol, unsigned int flags)
>  {
>  	if ((unsigned int)protocol < MAX_LINKS)
> --- wireless-dev.orig/include/linux/netlink.h	2007-07-03 00:09:13.047889695 +0200
> +++ wireless-dev/include/linux/netlink.h	2007-07-03 00:31:30.267889695 +0200
> @@ -161,6 +161,7 @@ extern struct sock *netlink_kernel_creat
>  					  void (*input)(struct sock *sk, int len),
>  					  struct mutex *cb_mutex,
>  					  struct module *module);
> +extern int netlink_change_ngroups(int unit, unsigned int groups);
>  extern void netlink_ack(struct sk_buff *in_skb, struct nlmsghdr *nlh, int err);
>  extern int netlink_has_listeners(struct sock *sk, unsigned int group);
>  extern int netlink_unicast(struct sock *ssk, struct sk_buff *skb, __u32 pid, int nonblock);
> 
> 

-
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