On Tue, Jan 27, 2009 at 09:06:51AM -0800, David Miller wrote:

From: Jarek Poplawski <redacted>
Date: Tue, 27 Jan 2009 12:31:11 +0000

quoted

On Tue, Jan 27, 2009 at 12:16:42PM +0000, Jarek Poplawski wrote:

quoted

On Tue, Jan 27, 2009 at 10:48:05PM +1100, Herbert Xu wrote:

quoted

On Tue, Jan 27, 2009 at 10:35:11AM +0000, Jarek Poplawski wrote:

quoted

quoted

quoted

Yes, but ip_append_data() (and skb_append_datato_frags() for
NETIF_F_UFO only, so currently not a problem), uses this differently,
and these pages in sk->sk_sndmsg_page could leak or be used after
kfree. (I didn't track locking in these other places).

It'll be freed when the socket is freed so that should be fine.

I don't think so: these places can overwrite sk->sk_sndmsg_page left
after tcp_sendmsg(), or skb_splice_bits() now, with NULL or a new
pointer without put_page() (they only reference copied chunks and
expect auto freeing). On the other hand, if tcp_sendmsg() reads after
them it could use a pointer after the page is freed, I guess.

I wasn't referring to the first part of your sentence.  That can't
happen because they're only used for UDP sockets, this is a TCP
socket.

Do you mean this part from ip_append_data() isn't used for TCP?:

Actually, the beginning part of ip_append_data() should be enough too.
So I guess I missed your point...

TCP doesn't use ip_append_data(), period.

Hmm... I see: TCP does use ip_send_reply(), so ip_append_data() too,
but with a special socket.

Thanks for the explanations,
Jarek P.