netdev - Re: [RFC 7/9] snet: introduce snet_netlink.c and snet

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: <m2ockvhmv1.fsf@ssh.synack.fr>
Date:	Fri, 15 Jan 2010 08:02:26 +0100
From:	Samir Bellabes <sam@...ack.fr>
To:	Patrick McHardy <kaber@...sh.net>
Cc:	linux-security-module@...r.kernel.org, jamal <hadi@...erus.ca>,
	Evgeniy Polyakov <zbr@...emap.net>,
	Neil Horman <nhorman@...driver.com>, netdev@...r.kernel.org,
	netfilter-devel@...r.kernel.org
Subject: Re: [RFC 7/9] snet: introduce snet_netlink.c and snet_netlink.h

Patrick McHardy <kaber@...sh.net> writes:

> Samir Bellabes wrote:
>> +/**
>> + * snet_nl_register - Handle a REGISTER message
>> + * @skb: the NETLINK buffer
>> + * @info: the Generic NETLINK info block
>> + *
>> + * Description:
>> + * Notify the kernel that an application is listening for events.
>> + * Returns zero on success, negative values on failure.
>> + */
>> +static int snet_nl_register(struct sk_buff *skb, struct genl_info *info)
>> +{
>> +	int ret = -EINVAL;
>> +	u32 version = 0;
>> +	u8 set_resp_flag = 0;
>> +
>> +	atomic_set(&snet_nl_seq, info->snd_seq);
>> +
>> +	if (!info->attrs[SNET_A_VERSION])
>> +		return -EINVAL;
>> +	version = nla_get_u32(info->attrs[SNET_A_VERSION]);
>> +
>> +	if (version == SNET_VERSION) {	/* version is compliant */
>> +		atomic_inc(&snet_num_listeners);
>> +		set_resp_flag = 1;
>> +	}
>> +
>> +	ret = snet_nl_response_flag(info, &snet_genl_family,
>> +				    SNET_C_REGISTER, SNET_A_REGISTERED,
>> +				    set_resp_flag);
>
> Is this really needed? A return value of 0 should already tell userspace
> that the command was successful. If it really wants a seperate success
> message, it can use NLM_F_ACK. This will also automatically take care
> of using the proper sequence number, so the snet_nl_seq handling isn't
> required anymore. Same for all similar cases below.

Indeed, we can use the return value to inform the userspace about the
status of operations. I moved all the code, and deleted the no longer
used function snet_nl_response_flag()

Thanks Patrick,
sam
--
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