netdev - Re: [PATCH 07/29] wimax: basic API: kernel/user messaging, rfkill and reset

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: <1228766499.22164.123.camel@johannes.berg>
Date:	Mon, 08 Dec 2008 21:01:39 +0100
From:	Johannes Berg <johannes@...solutions.net>
To:	Inaky Perez-Gonzalez <inaky@...ux.intel.com>
Cc:	netdev@...r.kernel.org, wimax@...uxwimax.org, greg@...ah.com
Subject: Re: [PATCH 07/29] wimax: basic API: kernel/user messaging, rfkill
	and reset

On Mon, 2008-12-08 at 11:09 -0800, Inaky Perez-Gonzalez wrote:

> +struct wimax_pipe *wimax_pipe_add(struct wimax_dev *wimax_dev,
> +				  const char *name)
> +{
> +	int result;
> +	struct device *dev = wimax_dev_to_dev(wimax_dev);
> +	struct wimax_pipe *pipe;
> +
> +	d_fnstart(3, dev, "(wimax_dev %p name %s)\n", wimax_dev, name);
> +	result = -ENOMEM;
> +	pipe = kzalloc(sizeof(*pipe), GFP_KERNEL);
> +	if (pipe == NULL) {
> +		dev_err(dev, "cannot allocate pipe '%s': %d\n",
> +			name, result);
> +		goto error_kzalloc;
> +	}

> +struct sk_buff *wimax_pipe_msg_alloc(struct wimax_dev *wimax_dev,
> +				     const void *msg, size_t size,
> +				     gfp_t gfp_flags)
> +{
> +	int result;
> +	struct device *dev = wimax_dev->net_dev->dev.parent;
> +	void *genl_msg;
> +	struct sk_buff *skb;
> +
> +	result = -ENOMEM;
> +	skb = genlmsg_new(nla_total_size(size), gfp_flags);
> +	if (skb == NULL)
> +		goto error_new;

> +int wimax_gnl_doit_msg_from_user(struct sk_buff *skb, struct genl_info *info)
> +{
> +	int result;
> +	struct wimax_dev *wimax_dev;
> +	struct device *dev;
> +	struct nlmsghdr *nlh = info->nlhdr;
> +	void *msg_buf;
> +	size_t msg_len;
> +
> +	might_sleep();
> +	d_fnstart(3, NULL, "(skb %p info %p)\n", skb, info);
> +	result = -EPERM;
> +	if (security_netlink_recv(skb, CAP_NET_ADMIN))
> +		goto error_perm;

perms check?

> +	result = -ENODEV;
> +	wimax_dev = wimax_dev_get_by_genl_info(info);
> +	if (wimax_dev == NULL)
> +		goto error_no_wimax_dev;


> +	result = wimax_dev->op_msg_from_user(wimax_dev, msg_buf, msg_len, info);
> +error_noop:
> +error_not_ready:
> +	mutex_unlock(&wimax_dev->mutex);
> +error_no_data:
> +	dev_put(wimax_dev->net_dev);
> +error_no_wimax_dev:
> +error_perm:
> +	d_fnend(3, NULL, "(skb %p info %p) = %d\n", skb, info, result);
> +	return result;

Do you really need all the fnstart/fnend debugging everywhere? Isn't
this easily covered by ftrace nowadays? If you remove that you can very
much simplify the code by using "return -ESOMETHING" instead of jumping
to a label in many of these functions.

johannes

Download attachment "signature.asc" of type "application/pgp-signature" (837 bytes)