From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mails.dpdk.org (mails.dpdk.org [217.70.189.124]) by inbox.dpdk.org (Postfix) with ESMTP id 26E27A0C41; Fri, 16 Apr 2021 09:21:01 +0200 (CEST) Received: from [217.70.189.124] (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id 1497A141AFE; Fri, 16 Apr 2021 09:21:01 +0200 (CEST) Received: from mail-wr1-f48.google.com (mail-wr1-f48.google.com [209.85.221.48]) by mails.dpdk.org (Postfix) with ESMTP id 94DF5141AE9 for ; Fri, 16 Apr 2021 09:20:59 +0200 (CEST) Received: by mail-wr1-f48.google.com with SMTP id g9so9641312wrx.0 for ; Fri, 16 Apr 2021 00:20:59 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=6wind.com; s=google; h=date:from:to:cc:subject:message-id:references:mime-version :content-disposition:in-reply-to:user-agent; bh=cXUpyCvJRu/8GXuQIsTjZvs5Nhv0QZ1cjioNACKqgKg=; b=TiXoBCBigr5mxf/BDLDEwXF9Enq/2tw30nT2LbTT59JGbJmxbyeoMQSUJqL6UrlntJ pNnv/RANiLe24op/c/+zfMLwftAEKnEysyC8p3XXr9WItXbRv4cL2bsaEnGKXXQi26Tn f7GYlruaX2FIl2vsk0hqdVnSPfkKW4RlO2lEowC0tChuQb699tRz8V217sw71rgvP0GC YiZ5kuxC/5D12jcJyYFigcWFkd3TWgnQxagqo4w7pgtkabbv+OHw+sKBdLpeIhsU/TXx HWXhnxuHRHXyAlZ5FkDx6HUbBH1MqiUsFR1NSZ/sA4CUfA61IDa/57LaymrlC11ottEp cyiQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:date:from:to:cc:subject:message-id:references :mime-version:content-disposition:in-reply-to:user-agent; bh=cXUpyCvJRu/8GXuQIsTjZvs5Nhv0QZ1cjioNACKqgKg=; b=r623FjqHQt1L6ABVPX6pJevTTrfcEjB8omaU6/rOPS6wgYpU7MGnpL2XSDJK4v4XWv EoVH6+AMHglIKJpWLGv9Ekw4CPDXvay16WVuMBPo8cn22Lbx61BXVs2YMDrq3RGadYMu pOjNCHNGL5S3K4eAxQ2Q93JGhRgYX3ptWc83SE9NpVWz1G5M4tlzVHvDaIobYAtelv9D 9X8fTAfq47ifpnqXGa0A0iTw+UzaO1OrzIUizJzH7p7EkvNrN/yhHN5EWBoBuwhPT3Az H+zPq5hh7FNMmwCYcoQMiD9tKZ9nLldJzk4I9rigDORwvLIfxs9KXfqUoUBNy/qIHx7V teNw== X-Gm-Message-State: AOAM531UXpYLhTyR6t2HAhJWlk1Q9B+Pq1YpN06kcASFRO08otebpR2y 4sQ+hEjGoDL9FuAnGtcMdDp5lA== X-Google-Smtp-Source: ABdhPJwAqkRezadPkYE+vzImpn1WXdBQP2LJNs6RFtSrvcxx5GmtRxXMh9oe6IOUG4YxP2aFZvTP2A== X-Received: by 2002:a5d:6ac6:: with SMTP id u6mr7527573wrw.290.1618557659365; Fri, 16 Apr 2021 00:20:59 -0700 (PDT) Received: from 6wind.com ([2a01:e0a:5ac:6460:c065:401d:87eb:9b25]) by smtp.gmail.com with ESMTPSA id s13sm8580170wrv.80.2021.04.16.00.20.58 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Fri, 16 Apr 2021 00:20:58 -0700 (PDT) Date: Fri, 16 Apr 2021 09:20:57 +0200 From: Olivier Matz To: Aaron Conole Cc: Thomas Monjalon , Wenwu Ma , andrew.rybchenko@oktetlabs.ru, stable@dpdk.org, dev@dpdk.org, zhihongx.peng@intel.com Message-ID: <20210416072057.GA1726@platinum> References: <20210331210557.4919-1-wenwux.ma@intel.com> <20210413200513.330399-1-wenwux.ma@intel.com> <2483269.apo0RDFAAg@thomas> <20210415064521.GE1650@platinum> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: User-Agent: Mutt/1.10.1 (2018-07-13) Subject: Re: [dpdk-dev] [dpdk-stable] [v2] test/mempool: fix heap buffer overflow X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org Sender: "dev" On Thu, Apr 15, 2021 at 08:51:27AM -0400, Aaron Conole wrote: > Olivier Matz writes: > > > On Tue, Apr 13, 2021 at 01:52:26PM +0200, Thomas Monjalon wrote: > >> 13/04/2021 22:05, Wenwu Ma: > >> > Amount of allocated memory was not enough for mempool > >> > which cause buffer overflow when access fields of mempool > >> > private structure in the rte_pktmbuf_priv_size function. > >> > >> Was it causing the test to fail? > >> How do you reproduce the overflow? > > > > In the test, right after the rte_mempool_create(), the function > > rte_mempool_obj_iter() is called too initialize the mempool objects with > > the rte_pktmbuf_init() callback function. This callback expects that the > > mempool is a packet pool, i.e. its private area is a struct > > rte_pktmbuf_pool_private structure. > > > > In the current test, the size of the private area is 0, which probably > > causes the function rte_pktmbuf_priv_size() to return an unpredictable > > value, and this value is used as a size in a memset. > > Is it possible to have rte_mempool_get_priv() detect that the private > area isn't valid and return a ref to a const static member for this that > will have the correct mbuf_priv_size? There isn't really documentation > that I can find that describes this corner case with the mempool private > data section. Actually, it doesn't really say what happens if private > data size is 0, so maybe a documentation update should go with this test > case fix, too? Good point, we can indeed add something in the API documentation. To detect that the private area is not big enough in rte_pktmbuf_init(), unfortunatly the function has no return code, but for now we can add at least an RTE_ASSERT() (only active when -DRTE_ENABLE_ASSERT is passed), as it's already done for other checks. I can do a new version of the patch. Wenwu, is it ok for you? In a second step, we can think about changing the API of all mempool callbacks and their wrappers to add a return code. > > This part of the test was added in commit 923ceaeac140 ("test/mempool: > > add unit test cases"). > > > > Instead of changing the size of the private area like done in the patch, > > I suggest to use another callback than rte_pktmbuf_init(). After all, > > this is a mempool test, so we should not rely on mbuf features. The > > function my_obj_init() could be used like in other places of the test, > > like this: > > > > @@ -552,7 +552,7 @@ test_mempool(void) > > GOTO_ERR(ret, err); > > > > /* test to initialize mempool objects and memory */ > > - nb_objs = rte_mempool_obj_iter(mp_stack_mempool_iter, rte_pktmbuf_init, > > + nb_objs = rte_mempool_obj_iter(mp_stack_mempool_iter, my_obj_init, > > NULL); > > if (nb_objs == 0) > > GOTO_ERR(ret, err); > > > > > > Wenwu, does it solve your issue? > > > > > > Regards, > > Olivier >