Message ID | 20201202085606.338429-1-stli@linux.ibm.com |
---|---|
State | Superseded |
Headers |
Return-Path: <libc-alpha-bounces@sourceware.org> X-Original-To: patchwork@sourceware.org Delivered-To: patchwork@sourceware.org Received: from server2.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id D1CB63959E5F; Wed, 2 Dec 2020 08:56:27 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org D1CB63959E5F DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=sourceware.org; s=default; t=1606899387; bh=7b6Svh5KpTgsNePm4v0W5Ei21cu3BUgVwOkstHFpRdU=; h=To:Subject:Date:List-Id:List-Unsubscribe:List-Archive:List-Post: List-Help:List-Subscribe:From:Reply-To:Cc:From; b=WS0n4a9Ak4Mky0wDDgiCOL9AX6Noc8XDBndfIt5bNKYNEVAg1SI3RRh6kiVn67Qwm JujcK+WhaWDFFftEpOr6P38MNoE5uw8qOBFrrxvnCiFEiMhLDP0FvrncKIJTDH3dgw p2AGoySL7eoRceJ6vpdpReMACgBr79PeBvjb0SvU= X-Original-To: libc-alpha@sourceware.org Delivered-To: libc-alpha@sourceware.org Received: from mx0b-001b2d01.pphosted.com (mx0b-001b2d01.pphosted.com [148.163.158.5]) by sourceware.org (Postfix) with ESMTPS id 0D8CB3846411 for <libc-alpha@sourceware.org>; Wed, 2 Dec 2020 08:56:26 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.3.2 sourceware.org 0D8CB3846411 Received: from pps.filterd (m0098421.ppops.net [127.0.0.1]) by mx0a-001b2d01.pphosted.com (8.16.0.42/8.16.0.42) with SMTP id 0B28kRbS186008 for <libc-alpha@sourceware.org>; Wed, 2 Dec 2020 03:56:25 -0500 Received: from ppma06ams.nl.ibm.com (66.31.33a9.ip4.static.sl-reverse.com [169.51.49.102]) by mx0a-001b2d01.pphosted.com with ESMTP id 355d9e4hbw-1 (version=TLSv1.2 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=NOT) for <libc-alpha@sourceware.org>; Wed, 02 Dec 2020 03:56:22 -0500 Received: from pps.filterd (ppma06ams.nl.ibm.com [127.0.0.1]) by ppma06ams.nl.ibm.com (8.16.0.42/8.16.0.42) with SMTP id 0B28q7I6024997 for <libc-alpha@sourceware.org>; Wed, 2 Dec 2020 08:56:21 GMT Received: from b06cxnps4074.portsmouth.uk.ibm.com (d06relay11.portsmouth.uk.ibm.com [9.149.109.196]) by ppma06ams.nl.ibm.com with ESMTP id 354fpdasqr-1 (version=TLSv1.2 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=NOT) for <libc-alpha@sourceware.org>; Wed, 02 Dec 2020 08:56:21 +0000 Received: from d06av26.portsmouth.uk.ibm.com (d06av26.portsmouth.uk.ibm.com [9.149.105.62]) by b06cxnps4074.portsmouth.uk.ibm.com (8.14.9/8.14.9/NCO v10.0) with ESMTP id 0B28uH997012980 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-GCM-SHA384 bits=256 verify=OK); Wed, 2 Dec 2020 08:56:17 GMT Received: from d06av26.portsmouth.uk.ibm.com (unknown [127.0.0.1]) by IMSVA (Postfix) with ESMTP id C9149AE045; Wed, 2 Dec 2020 08:56:17 +0000 (GMT) Received: from d06av26.portsmouth.uk.ibm.com (unknown [127.0.0.1]) by IMSVA (Postfix) with ESMTP id A94A6AE04D; Wed, 2 Dec 2020 08:56:17 +0000 (GMT) Received: from t35lp56.lnxne.boe (unknown [9.152.108.100]) by d06av26.portsmouth.uk.ibm.com (Postfix) with ESMTP; Wed, 2 Dec 2020 08:56:17 +0000 (GMT) To: libc-alpha@sourceware.org Subject: [PATCH] Handle out-of-memory case in svc_tcp.c/svc_unix.c:rendezvous_request. Date: Wed, 2 Dec 2020 09:56:06 +0100 Message-Id: <20201202085606.338429-1-stli@linux.ibm.com> X-Mailer: git-send-email 2.23.0 MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-TM-AS-GCONF: 00 X-Proofpoint-Virus-Version: vendor=fsecure engine=2.50.10434:6.0.312, 18.0.737 definitions=2020-12-02_01:2020-11-30, 2020-12-02 signatures=0 X-Proofpoint-Spam-Details: rule=outbound_notspam policy=outbound score=0 priorityscore=1501 impostorscore=0 lowpriorityscore=0 phishscore=0 mlxlogscore=313 clxscore=1015 mlxscore=0 malwarescore=0 spamscore=0 bulkscore=0 adultscore=0 suspectscore=1 classifier=spam adjust=0 reason=mlx scancount=1 engine=8.12.0-2009150000 definitions=main-2012020044 X-Spam-Status: No, score=-10.9 required=5.0 tests=BAYES_00, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_EF, GIT_PATCH_0, RCVD_IN_DNSWL_NONE, RCVD_IN_MSPIKE_H2, SPF_HELO_NONE, SPF_PASS, TXREP autolearn=ham autolearn_force=no version=3.4.2 X-Spam-Checker-Version: SpamAssassin 3.4.2 (2018-09-13) on server2.sourceware.org X-BeenThere: libc-alpha@sourceware.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Libc-alpha mailing list <libc-alpha.sourceware.org> List-Unsubscribe: <https://sourceware.org/mailman/options/libc-alpha>, <mailto:libc-alpha-request@sourceware.org?subject=unsubscribe> List-Archive: <https://sourceware.org/pipermail/libc-alpha/> List-Post: <mailto:libc-alpha@sourceware.org> List-Help: <mailto:libc-alpha-request@sourceware.org?subject=help> List-Subscribe: <https://sourceware.org/mailman/listinfo/libc-alpha>, <mailto:libc-alpha-request@sourceware.org?subject=subscribe> From: Stefan Liebler via Libc-alpha <libc-alpha@sourceware.org> Reply-To: Stefan Liebler <stli@linux.ibm.com> Cc: Stefan Liebler <stli@linux.ibm.com> Errors-To: libc-alpha-bounces@sourceware.org Sender: "Libc-alpha" <libc-alpha-bounces@sourceware.org> |
Series |
Handle out-of-memory case in svc_tcp.c/svc_unix.c:rendezvous_request.
|
|
Commit Message
Stefan Liebler
Dec. 2, 2020, 8:56 a.m. UTC
If glibc is build with -O3 on at least 390 (-m31) or x86 (-m32), gcc 11 dumps this warning: svc_tcp.c: In function 'rendezvous_request': svc_tcp.c:274:3: error: 'memcpy' offset [0, 15] is out of the bounds [0, 0] [-Werror=array-bounds] 274 | memcpy (&xprt->xp_raddr, &addr, sizeof (addr)); | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ cc1: all warnings being treated as errors In out-of-memory case, if one of the mallocs in makefd_xprt function returns NULL, a message is dumped, makefd_xprt returns NULL and the subsequent memcpy would copy to NULL. Instead of a segfaulting, svctcp_rendezvous_abort is now called. The same applies to svc_unix.c. --- sunrpc/svc_tcp.c | 5 +++++ sunrpc/svc_unix.c | 5 +++++ 2 files changed, 10 insertions(+)
Comments
On 02/12/2020 05:56, Stefan Liebler via Libc-alpha wrote: > If glibc is build with -O3 on at least 390 (-m31) or x86 (-m32), > gcc 11 dumps this warning: > svc_tcp.c: In function 'rendezvous_request': > svc_tcp.c:274:3: error: 'memcpy' offset [0, 15] is out of the bounds [0, 0] [-Werror=array-bounds] > 274 | memcpy (&xprt->xp_raddr, &addr, sizeof (addr)); > | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ > cc1: all warnings being treated as errors > > In out-of-memory case, if one of the mallocs in makefd_xprt function > returns NULL, a message is dumped, makefd_xprt returns NULL > and the subsequent memcpy would copy to NULL. > > Instead of a segfaulting, svctcp_rendezvous_abort is now called. It does not do what other parts of sunrpc does in case of memory allocation failure, it seems that usually the idea is to do some cleanup and return FALSE (for the case if the function returns bool_t). > > The same applies to svc_unix.c. > --- > sunrpc/svc_tcp.c | 5 +++++ > sunrpc/svc_unix.c | 5 +++++ > 2 files changed, 10 insertions(+) > > diff --git a/sunrpc/svc_tcp.c b/sunrpc/svc_tcp.c > index efbdd22548..738d47edb0 100644 > --- a/sunrpc/svc_tcp.c > +++ b/sunrpc/svc_tcp.c > @@ -271,6 +271,11 @@ again: > * make a new transporter (re-uses xprt) > */ > xprt = makefd_xprt (sock, r->sendsize, r->recvsize); > + > + /* If we are out of memory, makefd_xprt has already dumped an error. */ > + if (xprt == NULL) > + svctcp_rendezvous_abort (); > + > memcpy (&xprt->xp_raddr, &addr, sizeof (addr)); > xprt->xp_addrlen = len; > return FALSE; /* there is never an rpc msg to be processed */ > diff --git a/sunrpc/svc_unix.c b/sunrpc/svc_unix.c > index e01afeabe6..b13a4cd282 100644 > --- a/sunrpc/svc_unix.c > +++ b/sunrpc/svc_unix.c > @@ -270,6 +270,11 @@ again: > memset (&in_addr, '\0', sizeof (in_addr)); > in_addr.sin_family = AF_UNIX; > xprt = makefd_xprt (sock, r->sendsize, r->recvsize); > + > + /* If we are out of memory, makefd_xprt has already dumped an error. */ > + if (xprt == NULL) > + svcunix_rendezvous_abort (); > + > memcpy (&xprt->xp_raddr, &in_addr, sizeof (in_addr)); > xprt->xp_addrlen = len; > return FALSE; /* there is never an rpc msg to be processed */ >
* Adhemerval Zanella via Libc-alpha: > On 02/12/2020 05:56, Stefan Liebler via Libc-alpha wrote: >> If glibc is build with -O3 on at least 390 (-m31) or x86 (-m32), >> gcc 11 dumps this warning: >> svc_tcp.c: In function 'rendezvous_request': >> svc_tcp.c:274:3: error: 'memcpy' offset [0, 15] is out of the bounds [0, 0] [-Werror=array-bounds] >> 274 | memcpy (&xprt->xp_raddr, &addr, sizeof (addr)); >> | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ >> cc1: all warnings being treated as errors >> >> In out-of-memory case, if one of the mallocs in makefd_xprt function >> returns NULL, a message is dumped, makefd_xprt returns NULL >> and the subsequent memcpy would copy to NULL. >> >> Instead of a segfaulting, svctcp_rendezvous_abort is now called. > > It does not do what other parts of sunrpc does in case of memory allocation > failure, it seems that usually the idea is to do some cleanup and return > FALSE (for the case if the function returns bool_t). I think returning FALSE would introduce a variant of CVE-2011-4609 (bug 14889). The sleeping added in commit 14bc93a967e62abf8cf2704725b6f7 for that is rather hackish (more correct would be to remove the accepting socket from the polling set until a client exits), but sleeping on ENOMEM might be a reasonable approach here, given that the sunrpc code is deep maintenance. Thanks, Florian
On 03/12/2020 18:03, Florian Weimer wrote: > * Adhemerval Zanella via Libc-alpha: > >> On 02/12/2020 05:56, Stefan Liebler via Libc-alpha wrote: >>> If glibc is build with -O3 on at least 390 (-m31) or x86 (-m32), >>> gcc 11 dumps this warning: >>> svc_tcp.c: In function 'rendezvous_request': >>> svc_tcp.c:274:3: error: 'memcpy' offset [0, 15] is out of the bounds [0, 0] [-Werror=array-bounds] >>> 274 | memcpy (&xprt->xp_raddr, &addr, sizeof (addr)); >>> | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ >>> cc1: all warnings being treated as errors >>> >>> In out-of-memory case, if one of the mallocs in makefd_xprt function >>> returns NULL, a message is dumped, makefd_xprt returns NULL >>> and the subsequent memcpy would copy to NULL. >>> >>> Instead of a segfaulting, svctcp_rendezvous_abort is now called. >> >> It does not do what other parts of sunrpc does in case of memory allocation >> failure, it seems that usually the idea is to do some cleanup and return >> FALSE (for the case if the function returns bool_t). > > I think returning FALSE would introduce a variant of CVE-2011-4609 (bug > 14889). The sleeping added in commit 14bc93a967e62abf8cf2704725b6f7 for > that is rather hackish (more correct would be to remove the accepting > socket from the polling set until a client exits), but sleeping on > ENOMEM might be a reasonable approach here, given that the sunrpc code > is deep maintenance. Using the sleep hack should be slight better and avoid ENOMEM to abort the process.
On 12/4/20 1:25 PM, Adhemerval Zanella via Libc-alpha wrote: > > > On 03/12/2020 18:03, Florian Weimer wrote: >> * Adhemerval Zanella via Libc-alpha: >> >>> On 02/12/2020 05:56, Stefan Liebler via Libc-alpha wrote: >>>> If glibc is build with -O3 on at least 390 (-m31) or x86 (-m32), >>>> gcc 11 dumps this warning: >>>> svc_tcp.c: In function 'rendezvous_request': >>>> svc_tcp.c:274:3: error: 'memcpy' offset [0, 15] is out of the bounds [0, 0] [-Werror=array-bounds] >>>> 274 | memcpy (&xprt->xp_raddr, &addr, sizeof (addr)); >>>> | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ >>>> cc1: all warnings being treated as errors >>>> >>>> In out-of-memory case, if one of the mallocs in makefd_xprt function >>>> returns NULL, a message is dumped, makefd_xprt returns NULL >>>> and the subsequent memcpy would copy to NULL. >>>> >>>> Instead of a segfaulting, svctcp_rendezvous_abort is now called. >>> >>> It does not do what other parts of sunrpc does in case of memory allocation >>> failure, it seems that usually the idea is to do some cleanup and return >>> FALSE (for the case if the function returns bool_t). >> >> I think returning FALSE would introduce a variant of CVE-2011-4609 (bug >> 14889). The sleeping added in commit 14bc93a967e62abf8cf2704725b6f7 for >> that is rather hackish (more correct would be to remove the accepting >> socket from the polling set until a client exits), but sleeping on >> ENOMEM might be a reasonable approach here, given that the sunrpc code >> is deep maintenance. > > Using the sleep hack should be slight better and avoid ENOMEM to abort > the process. > Now it sleeps and FALSE is returned. Please have a look at: "[PATCH v2] Handle out-of-memory case in svc_tcp.c/svc_unix.c:rendezvous_request." https://sourceware.org/pipermail/libc-alpha/2020-December/120410.html Thanks, Stefan
diff --git a/sunrpc/svc_tcp.c b/sunrpc/svc_tcp.c index efbdd22548..738d47edb0 100644 --- a/sunrpc/svc_tcp.c +++ b/sunrpc/svc_tcp.c @@ -271,6 +271,11 @@ again: * make a new transporter (re-uses xprt) */ xprt = makefd_xprt (sock, r->sendsize, r->recvsize); + + /* If we are out of memory, makefd_xprt has already dumped an error. */ + if (xprt == NULL) + svctcp_rendezvous_abort (); + memcpy (&xprt->xp_raddr, &addr, sizeof (addr)); xprt->xp_addrlen = len; return FALSE; /* there is never an rpc msg to be processed */ diff --git a/sunrpc/svc_unix.c b/sunrpc/svc_unix.c index e01afeabe6..b13a4cd282 100644 --- a/sunrpc/svc_unix.c +++ b/sunrpc/svc_unix.c @@ -270,6 +270,11 @@ again: memset (&in_addr, '\0', sizeof (in_addr)); in_addr.sin_family = AF_UNIX; xprt = makefd_xprt (sock, r->sendsize, r->recvsize); + + /* If we are out of memory, makefd_xprt has already dumped an error. */ + if (xprt == NULL) + svcunix_rendezvous_abort (); + memcpy (&xprt->xp_raddr, &in_addr, sizeof (in_addr)); xprt->xp_addrlen = len; return FALSE; /* there is never an rpc msg to be processed */