From mboxrd@z Thu Jan 1 00:00:00 1970 From: Tom Herbert Subject: Re: [PATCH net] strparser: Fix sign of err codes Date: Mon, 26 Mar 2018 13:44:44 -0700 Message-ID: References: <20180326193121.GA78356@davejwatson-mba.local> Mime-Version: 1.0 Content-Type: text/plain; charset="UTF-8" Cc: "David S. Miller" , Linux Kernel Network Developers To: Dave Watson Return-path: Received: from mail-wr0-f196.google.com ([209.85.128.196]:34255 "EHLO mail-wr0-f196.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751076AbeCZUoq (ORCPT ); Mon, 26 Mar 2018 16:44:46 -0400 Received: by mail-wr0-f196.google.com with SMTP id o8so20236132wra.1 for ; Mon, 26 Mar 2018 13:44:45 -0700 (PDT) In-Reply-To: <20180326193121.GA78356@davejwatson-mba.local> Sender: netdev-owner@vger.kernel.org List-ID: On Mon, Mar 26, 2018 at 12:31 PM, Dave Watson wrote: > 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. > Nice catch! It might be nice to have a comment at strp_parser_err and abort_parser description in Documentation/networking/strparser.txt should also be updated that err is a negative error value. Tom > Fixes: 43a0c6751a322847 ("strparser: Stream parser for messages") > Signed-off-by: Dave Watson > --- > net/strparser/strparser.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/net/strparser/strparser.c b/net/strparser/strparser.c > index 1fdab5c..b9283ce 100644 > --- a/net/strparser/strparser.c > +++ b/net/strparser/strparser.c > @@ -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); > } > } > @@ -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 >