netdev - Re: [PATCH 2/2] tcp: cleanup tcp_try

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: <4FA22F07.6060306@gmail.com>
Date:	Thu, 03 May 2012 00:08:55 -0700
From:	Alexander Duyck <alexander.duyck@...il.com>
To:	David Miller <davem@...emloft.net>
CC:	eric.dumazet@...il.com, alexander.h.duyck@...el.com,
	netdev@...r.kernel.org, edumazet@...gle.com,
	jeffrey.t.kirsher@...el.com
Subject: Re: [PATCH 2/2] tcp: cleanup tcp_try_coalesce

On 5/2/2012 10:50 PM, David Miller wrote:
> From: Alexander Duyck<alexander.duyck@...il.com>
> Date: Wed, 02 May 2012 22:41:36 -0700
>
>> I think the part that has me confused is how being more precise about
>> removing from truesize gets in the way of detecting abuses of
>> truesize.  It seems like it should be more as good as or better then
>> the original approach of just using skb->len.
> You can only trim from truesize if you can be absolutely certain that
> you have removed any reference in the fraglist, or the page'd head, to
> the entire "block" of data.
>
> If the skb still refers to even just one byte in a particular block,
> we must still charge the entire block in the truesize.
I get that, and that is what my code was doing that the old code wasn't 
doing.  That is why I am confused.  I should have another patch ready in 
an hour or so that should help to make my position clear.

> For example, NIU has three pools of power-of-2 blocks of data it
> maintainers and the device pulls from to build incoming packets.
>
> So if the chip used one of the 2048 byte buffers, we charge the entire
> 2048 bytes even of the packet is much smaller.
>
> Conversely this means we cannot trim the 2048 part of the truesize of
> that SKB unless we had some mechanism to know for certain 1) what the
> block size of the underlying data is and 2) that we've removed all
> references to that.
>
> Currently this is not really possible, so we therefore defer truesize
> adjustments.
This is true for the frags, but not for the head buffer allocated with 
__alloc_skb or build_skb.  For either of these we set both truesize and 
the end pointer offset based on the ksize(data).  The truesize is the 
value plus SKB_DATA_ALIGNED(sizeof(struct sk_buff)), and the end pointer 
offset is the value minus SKB_DATA_ALIGNED(sizeof(struct 
skb_shared_info)).  So in the case where we are removing the sk_buff and 
skb->head, I am subtracting the end pointer offset plus the aligned 
shared info and sk_buff values from skb->truesize to get the size of 
just the paged region.

> Behaving otherwise is dangerous, because then we'd potentially end up
> with a lot of memory used, but not actually accounted for by anyone.
I fully agree that is why I was surprised when I was told not to use the 
values for skb->truesize that were computed back when we allocated the 
skb to determine the truesize to remove from the delta when we were 
removing it.

Thanks,

Alex
--
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