netdev - Re: [PATCH 1/4] [UDP]: fix send buffer check

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: <20071220.033138.157714241.davem@davemloft.net>
Date:	Thu, 20 Dec 2007 03:31:38 -0800 (PST)
From:	David Miller <davem@...emloft.net>
To:	haoki@...hat.com
Cc:	herbert@...dor.apana.org.au, netdev@...r.kernel.org,
	tyasui@...hat.com, mhiramat@...hat.com,
	satoshi.oshima.fk@...achi.com, billfink@...dspring.com,
	andi@...stfloor.org, johnpol@....mipt.ru,
	shemminger@...ux-foundation.org, yoshfuji@...ux-ipv6.org,
	yumiko.sugita.yf@...achi.com
Subject: Re: [PATCH 1/4] [UDP]: fix send buffer check

From: Hideo AOKI <haoki@...hat.com>
Date: Mon, 17 Dec 2007 21:38:03 -0500

> This patch introduces sndbuf size check before memory allocation for
> send buffer.
> 
> signed-off-by: Satoshi Oshima <satoshi.oshima.fk@...achi.com>
> signed-off-by: Hideo Aoki <haoki@...hat.com>
 ...
> diff -pruN net-2.6/net/ipv4/ip_output.c net-2.6-udp-take11a1-p1/net/ipv4/ip_output.c
> --- net-2.6/net/ipv4/ip_output.c	2007-12-11 10:54:55.000000000 -0500
> +++ net-2.6-udp-take11a1-p1/net/ipv4/ip_output.c	2007-12-17 14:42:31.000000000 -0500
> @@ -1004,6 +1004,11 @@ alloc_new_skb:
>  					frag = &skb_shinfo(skb)->frags[i];
>  				}
>  			} else if (i < MAX_SKB_FRAGS) {
> +				if (atomic_read(&sk->sk_wmem_alloc) + PAGE_SIZE
> +				    > 2 * sk->sk_sndbuf) {
> +					err = -ENOBUFS;
> +					goto error;
> +				}
>  				if (copy > PAGE_SIZE)
>  					copy = PAGE_SIZE;
>  				page = alloc_pages(sk->sk_allocation, 0);

If we are going to do this, we need to add the same check to
skb_append_datato_frags() which is invoked via ip_ufo_append_data().

We also have to be very careful in this area.  One problem we had a
long time ago was that we would socket account when fragmenting an
outgoing frame.  This was bogus because even if the socket had enough
space for one full sized frame, the packet send would fail because it
could not fit the space for both the original frame and the
fragmented copy of it.

This situation was cured by simply not enforcing accounting for the
fragmented copy.  It is valid because after we fragment, we keep
the fragmented copy but free the original.

This doesn't apply directly to this specific patch, but it is
something to keep in mind when doing these changes.
--
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