netdev - [PATCH v2 net] strparser: Fix sign of err codes

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-next>] [day] [month] [year] [list]

Message-ID: <20180327152352.GA35552@davejwatson-mba.local>
Date:   Tue, 27 Mar 2018 08:23:52 -0700
From:   Dave Watson <davejwatson@...com>
To:     "David S. Miller" <davem@...emloft.net>,
        Tom Herbert <tom@...ntonium.net>, <netdev@...r.kernel.org>
Subject: [PATCH v2 net] strparser: Fix sign of err codes

strp_parser_err is called with a negative code everywhere, which then
calls abort_parser with a negative code.  strp_msg_timeout calls
abort_parser directly with a positive code.  Negate ETIMEDOUT
to match signed-ness of other calls.

The default abort_parser callback, strp_abort_strp, sets
sk->sk_err to err.  Also negate the error here so sk_err always
holds a positive value, as the rest of the net code expects.  Currently
a negative sk_err can result in endless loops, or user code that
thinks it actually sent/received err bytes.

Found while testing net/tls_sw recv path.

Fixes: 43a0c6751a322847 ("strparser: Stream parser for messages")
Signed-off-by: Dave Watson <davejwatson@...com>
---
 Documentation/networking/strparser.txt | 5 +++--
 net/strparser/strparser.c              | 8 ++++----
 2 files changed, 7 insertions(+), 6 deletions(-)

diff --git a/Documentation/networking/strparser.txt b/Documentation/networking/strparser.txt
index 13081b3..23bf827 100644
--- a/Documentation/networking/strparser.txt
+++ b/Documentation/networking/strparser.txt
@@ -158,7 +158,8 @@ int (*read_sock_done)(struct strparser *strp, int err);
      the TCP socket in receive callback mode. The stream parser may
      read multiple messages in a loop and this function allows cleanup
      to occur when exiting the loop. If the callback is not set (NULL
-     in strp_init) a default function is used.
+     in strp_init) a default function is used.  err is a negative
+     error value.
 
 void (*abort_parser)(struct strparser *strp, int err);
 
@@ -166,7 +167,7 @@ void (*abort_parser)(struct strparser *strp, int err);
      in parsing. The default function stops the stream parser and
      sets the error in the socket if the parser is in receive callback
      mode. The default function can be changed by setting the callback
-     to non-NULL in strp_init.
+     to non-NULL in strp_init. err is a negative error value.
 
 Statistics
 ==========
diff --git a/net/strparser/strparser.c b/net/strparser/strparser.c
index 1fdab5c..a82ca09 100644
--- a/net/strparser/strparser.c
+++ b/net/strparser/strparser.c
@@ -44,7 +44,7 @@ static inline struct _strp_msg *_strp_msg(struct sk_buff *skb)
 		offsetof(struct qdisc_skb_cb, data));
 }
 
-/* Lower lock held */
+/* Lower lock held.  err is a negative error value. */
 static void strp_abort_strp(struct strparser *strp, int err)
 {
 	/* Unrecoverable error in receive */
@@ -60,7 +60,7 @@ static void strp_abort_strp(struct strparser *strp, int err)
 		struct sock *sk = strp->sk;
 
 		/* Report an error on the lower socket */
-		sk->sk_err = err;
+		sk->sk_err = -err;
 		sk->sk_error_report(sk);
 	}
 }
@@ -71,7 +71,7 @@ static void strp_start_timer(struct strparser *strp, long timeo)
 		mod_delayed_work(strp_wq, &strp->msg_timer_work, timeo);
 }
 
-/* Lower lock held */
+/* Lower lock held.  err is a negative error value. */
 static void strp_parser_err(struct strparser *strp, int err,
 			    read_descriptor_t *desc)
 {
@@ -458,7 +458,7 @@ static void strp_msg_timeout(struct work_struct *w)
 	/* Message assembly timed out */
 	STRP_STATS_INCR(strp->stats.msg_timeouts);
 	strp->cb.lock(strp);
-	strp->cb.abort_parser(strp, ETIMEDOUT);
+	strp->cb.abort_parser(strp, -ETIMEDOUT);
 	strp->cb.unlock(strp);
 }
 
-- 
2.9.5