netdev - Re: [PATCH net-next 04/14] tipc: add sock dump to new netlink api

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 for Android: free password hash cracker in your pocket

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

Message-Id: <20140912.171038.1165432718811920305.davem@davemloft.net>
Date:	Fri, 12 Sep 2014 17:10:38 -0400 (EDT)
From:	David Miller <davem@...emloft.net>
To:	richard.alpe@...csson.com
Cc:	netdev@...r.kernel.org, tipc-discussion@...ts.sourceforge.net
Subject: Re: [PATCH net-next 04/14] tipc: add sock dump to new netlink api

From: <richard.alpe@...csson.com>
Date: Thu, 11 Sep 2014 10:29:17 +0200

> +	list_for_each_entry_from(p, &tsk->publications, pport_list) {
> +		publ = nla_nest_start(skb, TIPC_NLA_SOCK_PUBL);
> +		if (nla_put_u32(skb, TIPC_NLA_PUBL_TYPE, p->type))
> +			goto msg_full;
> +		if (nla_put_u32(skb, TIPC_NLA_PUBL_LOWER, p->lower))
> +			goto msg_full;
> +		if (nla_put_u32(skb, TIPC_NLA_PUBL_UPPER, p->upper))
> +			goto msg_full;
> +		nla_nest_end(skb, publ);
> +	}
> +
> +	*prev_publ = 0;
> +
> +	return 0;
> +
> +msg_full:
> +	*prev_publ = p->key;
> +	nla_nest_cancel(skb, publ);

This restart mechanism is broken.

You can't public nested information this way.

What happens in your code is that if we hit the limit in the middle of
adding the publications, the next time we'll put the same socket into
the netlink message and then the rest of the nested publications.
That's malformed.

You can't just say sometimes you'll partially list the set of nested
attributes in an object, you must public the entire object fully in
the netlink message or skip the object entirely.

I would suggest that you instead size the amount of space you'll
need for at least the first socket being listed, and if NLMSG_GOODSIZE
is insufficient, allocate as much as you will actually need.

Then you put full socket netlink blobs in there, including all nested
attributes, and then stop and reset back the the most recent full socket
published if you run out of space.
--
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