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: <m2hbqqilsm.fsf@ssh.synack.fr>
Date:	Wed, 13 Jan 2010 07:03:21 +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:
>> +
>> +	msg_head = genlmsg_put(skb_rsp, snet_nl_pid,
>> +			       atomic_inc_return(&snet_nl_seq),
>> +			       &snet_genl_family, 0, SNET_C_VERDICT);
>> +	if (msg_head == NULL)
>> +		goto send_event_failure;
>> +
>> +	snet_dbg("verdict_id=0x%x syscall=%s protocol=%u "
>> +		 "family=%u uid=%u pid=%u\n",
>> +		 verdict_id, snet_syscall_name(syscall),
>> +		 protocol, family, current_uid(), current->pid);
>> +
>> +	if (verdict_id) {
>> +		ret = nla_put_u32(skb_rsp, SNET_A_VERDICT_ID, verdict_id);
>> +		if (ret != 0)
>> +			goto send_event_failure;
>> +	}
>> +	ret = nla_put_u8(skb_rsp, SNET_A_SYSCALL, syscall);
>> +	if (ret != 0)
>> +		goto send_event_failure;
>> +	ret = nla_put_u8(skb_rsp, SNET_A_PROTOCOL, protocol);
>> +	if (ret != 0)
>> +		goto send_event_failure;
>> +	ret = nla_put_u8(skb_rsp, SNET_A_FAMILY, family);
>> +	if (ret != 0)
>> +		goto send_event_failure;
>> +	ret = nla_put_u32(skb_rsp, SNET_A_UID, current_uid());
>> +	if (ret != 0)
>> +		goto send_event_failure;
>> +	ret = nla_put_u32(skb_rsp, SNET_A_PID, current->pid);
>> +	if (ret != 0)
>> +		goto send_event_failure;
>> +	ret = nla_put(skb_rsp, SNET_A_DATA_EXT, len, data);
>> +	if (ret != 0)
>> +		goto send_event_failure;
>
> I guess its a matter of taste, but the NLA_PUT macros already take
> care of exception handling and keep the code smaller.

indeed, I moved the code to use NLA_PUT and the hidden failure label

>> +
>> +	ret = genlmsg_end(skb_rsp, msg_head);
>> +	if (ret < 0)
>> +		goto send_event_failure;
>> +
>> +	genlmsg_unicast(&init_net, skb_rsp, snet_nl_pid);
>> +	return 0;
>> +
>> +send_event_failure:
>> +	kfree_skb(skb_rsp);
>> +	return 0;
>
> Shouldn't this error be returned to the caller to avoid waiting
> for the timeout to occur (same question for the return value of
> genlmsg_unicast, which can also fail).

indeed, if error occurred, no need to wait, we can apply the polocy
verdict.
genlmsg_unicast() is taking care of freeing the skb if error occured, so
we can return directly the value of genlmsg_unicast()

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