2015-05-18 18:36:24

by Eric B Munson

[permalink] [raw]
Subject: [PATCH net-next] tcp: Return error instead of partial read for saved syn headers

Currently the getsockopt() requesting the cached contents of the syn
packet headers will fail silently if the caller uses a buffer that is
too small to contain the requested data. Rather than fail silently and
discard the headers, getsockopt() should return an error and report the
required size to hold the data.

Signed-off-by: Eric B Munson <[email protected]>
Cc: Eric Dumazet <[email protected]>
Cc: Alexey Kuznetsov <[email protected]>
Cc: James Morris <[email protected]>
Cc: Hideaki YOSHIFUJI <[email protected]>
Cc: Patrick McHardy <[email protected]>
Cc: [email protected]
Cc: [email protected]
---
net/ipv4/tcp.c | 10 +++++++++-
1 file changed, 9 insertions(+), 1 deletion(-)

diff --git a/net/ipv4/tcp.c b/net/ipv4/tcp.c
index c724195..bb9bb84 100644
--- a/net/ipv4/tcp.c
+++ b/net/ipv4/tcp.c
@@ -2845,7 +2845,15 @@ static int do_tcp_getsockopt(struct sock *sk, int level,

lock_sock(sk);
if (tp->saved_syn) {
- len = min_t(unsigned int, tp->saved_syn[0], len);
+ if (len < tp->saved_syn[0]) {
+ if (put_user(tp->saved_syn[0], optlen)) {
+ release_sock(sk);
+ return -EFAULT;
+ }
+ release_sock(sk);
+ return -EINVAL;
+ }
+ len = tp->saved_syn[0];
if (put_user(len, optlen)) {
release_sock(sk);
return -EFAULT;
--
1.9.1


2015-05-18 18:41:49

by Rick Jones

[permalink] [raw]
Subject: Re: [PATCH net-next] tcp: Return error instead of partial read for saved syn headers

On 05/18/2015 11:35 AM, Eric B Munson wrote:
> Currently the getsockopt() requesting the cached contents of the syn
> packet headers will fail silently if the caller uses a buffer that is
> too small to contain the requested data. Rather than fail silently and
> discard the headers, getsockopt() should return an error and report the
> required size to hold the data.

Is there any chapter and verse on whether a "failed" getsockopt() may
alter the items passed to it?

rick jones

2015-05-18 19:01:25

by Eric Dumazet

[permalink] [raw]
Subject: Re: [PATCH net-next] tcp: Return error instead of partial read for saved syn headers

On Mon, 2015-05-18 at 11:41 -0700, Rick Jones wrote:
> On 05/18/2015 11:35 AM, Eric B Munson wrote:
> > Currently the getsockopt() requesting the cached contents of the syn
> > packet headers will fail silently if the caller uses a buffer that is
> > too small to contain the requested data. Rather than fail silently and
> > discard the headers, getsockopt() should return an error and report the
> > required size to hold the data.
>
> Is there any chapter and verse on whether a "failed" getsockopt() may
> alter the items passed to it?

This should be fine.

getsockopt() has two copyout to perform, the second one can fail.

We can not 'undo' the first one in a safe way.

2015-05-19 20:33:54

by David Miller

[permalink] [raw]
Subject: Re: [PATCH net-next] tcp: Return error instead of partial read for saved syn headers

From: Eric B Munson <[email protected]>
Date: Mon, 18 May 2015 14:35:58 -0400

> Currently the getsockopt() requesting the cached contents of the syn
> packet headers will fail silently if the caller uses a buffer that is
> too small to contain the requested data. Rather than fail silently and
> discard the headers, getsockopt() should return an error and report the
> required size to hold the data.
>
> Signed-off-by: Eric B Munson <[email protected]>

Applied, thanks Eric.