From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-8.2 required=3.0 tests=HEADER_FROM_DIFFERENT_DOMAINS, INCLUDES_PATCH,MAILING_LIST_MULTI,SIGNED_OFF_BY,SPF_HELO_NONE,SPF_PASS, USER_AGENT_SANE_1 autolearn=unavailable autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id D6955C433DF for ; Thu, 11 Jun 2020 14:13:19 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id BB5D12083E for ; Thu, 11 Jun 2020 14:13:19 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1726976AbgFKONL (ORCPT ); Thu, 11 Jun 2020 10:13:11 -0400 Received: from www62.your-server.de ([213.133.104.62]:55442 "EHLO www62.your-server.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1727768AbgFKONJ (ORCPT ); Thu, 11 Jun 2020 10:13:09 -0400 Received: from sslproxy02.your-server.de ([78.47.166.47]) by www62.your-server.de with esmtpsa (TLSv1.2:DHE-RSA-AES256-GCM-SHA384:256) (Exim 4.89_1) (envelope-from ) id 1jjNwr-0006SL-06; Thu, 11 Jun 2020 16:12:53 +0200 Received: from [178.196.57.75] (helo=pc-9.home) by sslproxy02.your-server.de with esmtpsa (TLSv1.3:TLS_AES_256_GCM_SHA384:256) (Exim 4.92) (envelope-from ) id 1jjNwq-000TE2-HS; Thu, 11 Jun 2020 16:12:52 +0200 Subject: Re: [PATCH] xdp_rxq_info_user: Add null check after malloc To: Gaurav Singh , Alexei Starovoitov , "David S. Miller" , Jakub Kicinski , Jesper Dangaard Brouer , John Fastabend , Martin KaFai Lau , Song Liu , Yonghong Song , Andrii Nakryiko , KP Singh , "open list:XDP (eXpress Data Path)" , "open list:XDP (eXpress Data Path)" , open list References: <20200610030145.17263-1-gaurav1086@gmail.com> From: Daniel Borkmann Message-ID: Date: Thu, 11 Jun 2020 16:12:51 +0200 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101 Thunderbird/60.7.2 MIME-Version: 1.0 In-Reply-To: <20200610030145.17263-1-gaurav1086@gmail.com> Content-Type: text/plain; charset=utf-8; format=flowed Content-Language: en-US Content-Transfer-Encoding: 7bit X-Authenticated-Sender: daniel@iogearbox.net X-Virus-Scanned: Clear (ClamAV 0.102.3/25840/Thu Jun 11 14:52:31 2020) Sender: bpf-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: bpf@vger.kernel.org On 6/10/20 5:01 AM, Gaurav Singh wrote: > Signed-off-by: Gaurav Singh > > The memset call is made right after malloc call which > can return a NULL pointer upon failure causing a > segmentation fault. Fix this by adding a null check > right after malloc() and then do memset(). The SoB should come after the commit message here. > --- > samples/bpf/xdp_rxq_info_user.c | 6 +++--- > 1 file changed, 3 insertions(+), 3 deletions(-) > > diff --git a/samples/bpf/xdp_rxq_info_user.c b/samples/bpf/xdp_rxq_info_user.c > index 4fe47502ebed..2d03c84a4cca 100644 > --- a/samples/bpf/xdp_rxq_info_user.c > +++ b/samples/bpf/xdp_rxq_info_user.c > @@ -202,11 +202,11 @@ static struct datarec *alloc_record_per_cpu(void) > > size = sizeof(struct datarec) * nr_cpus; > array = malloc(size); > - memset(array, 0, size); All these below are candidates for calloc(), can we just use that instead and simplify the below. > if (!array) { > fprintf(stderr, "Mem alloc error (nr_cpus:%u)\n", nr_cpus); > exit(EXIT_FAIL_MEM); > } > + memset(array, 0, size); > return array; > } > > @@ -218,11 +218,11 @@ static struct record *alloc_record_per_rxq(void) > > size = sizeof(struct record) * nr_rxqs; > array = malloc(size); > - memset(array, 0, size); > if (!array) { > fprintf(stderr, "Mem alloc error (nr_rxqs:%u)\n", nr_rxqs); > exit(EXIT_FAIL_MEM); > } > + memset(array, 0, size); > return array; > } > > @@ -233,11 +233,11 @@ static struct stats_record *alloc_stats_record(void) > int i; > > rec = malloc(sizeof(*rec)); > - memset(rec, 0, sizeof(*rec)); > if (!rec) { > fprintf(stderr, "Mem alloc error\n"); > exit(EXIT_FAIL_MEM); > } > + memset(rec, 0, sizeof(*rec)); > rec->rxq = alloc_record_per_rxq(); > for (i = 0; i < nr_rxqs; i++) > rec->rxq[i].cpu = alloc_record_per_cpu(); > Thanks, Daniel