From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-wm0-f46.google.com (mail-wm0-f46.google.com [74.125.82.46]) by dpdk.org (Postfix) with ESMTP id 773B33977 for ; Tue, 2 Aug 2016 11:58:57 +0200 (CEST) Received: by mail-wm0-f46.google.com with SMTP id i5so281758410wmg.0 for ; Tue, 02 Aug 2016 02:58:57 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=6wind-com.20150623.gappssmtp.com; s=20150623; h=date:from:to:cc:subject:message-id:mail-followup-to:references :mime-version:content-disposition:in-reply-to; bh=tsdbGOtBddY0hu8xvOcQFnIpIoCwqq9HDL5zXjHbShM=; b=t4C0sg5HwZjV38vhRCyw9O7fMrvIspJHOaORj9AzL+aY/uAMPBFn1HEfBHOqScS7sJ T/VN5WftpWY2KSSetrE8+Vqjda3M7mmgk13a/bSMtD0mra1DKliPtKJSiv/ROll1oti8 jZBEjxFvZfi6GXPDGe74Col0O5I2x9lgcOFLUF0ChRyPvmovUQoLBSplDBSUctc8mEFF iWtz1vyU3UMmXtw3Hi0ttoIuhjH0qhulKpJYMDfpnWPmuupOHUBufnLoacq5qKGwPXJK F+Al9meFEXauGdFBoULY0uU0jEtlPDAZFsG7l8l3nYTGSLgLCfOIIlxoo5aJzFgfXO7g p+Ww== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:date:from:to:cc:subject:message-id :mail-followup-to:references:mime-version:content-disposition :in-reply-to; bh=tsdbGOtBddY0hu8xvOcQFnIpIoCwqq9HDL5zXjHbShM=; b=TlLjSaw/a+DrWhYSr3XLpVvwlyVw4iEmHODAu6FJcaJrE89hYcCWwWJCQDQ6lzXQQE lpCAI1hmKn6Nnw02vHd5YmgjBg/bacH7LNYDnFgyMRgp7y2RkRamGuvZQ46JTe5/KJ2j J+g4c+kRO+1/roVhWGhNl5Fl5lRyaTtWlN/kd+NlAKgkyoLsPnv29+fnCPjurzV9kx7t Tw1jkbq/JuYoGVZBuI2jt2VdOjZbias2LzjWPqH+35X94UzklG6fJnDSO1I02taLpYB+ nX5ORmavaRTZNehJkbqvrD46mVHVhmFBB92XGxX4Yf5vr0Vj+VJWbIyK9/TaEmYtuaAL VblQ== X-Gm-Message-State: AEkoousS9kaqNa0CFwVKBTux0ysQK7FoL45ePY5JwbaQTKupCdDDbY0yAb2rzIfs+EPKvzz7 X-Received: by 10.194.112.166 with SMTP id ir6mr60607062wjb.95.1470131937155; Tue, 02 Aug 2016 02:58:57 -0700 (PDT) Received: from 6wind.com (guy78-3-82-239-227-177.fbx.proxad.net. [82.239.227.177]) by smtp.gmail.com with ESMTPSA id yz6sm1785505wjb.35.2016.08.02.02.58.55 (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Tue, 02 Aug 2016 02:58:56 -0700 (PDT) Date: Tue, 2 Aug 2016 11:58:52 +0200 From: Adrien Mazarguil To: Sagi Grimberg Cc: dev@dpdk.org Message-ID: <20160802095852.GB30580@6wind.com> Mail-Followup-To: Sagi Grimberg , dev@dpdk.org References: <1470041061-8059-1-git-send-email-sagi@grimberg.me> <20160801164342.GL9044@6wind.com> <0e002bcc-017b-8d5e-f820-111f5c3a7b46@grimberg.me> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <0e002bcc-017b-8d5e-f820-111f5c3a7b46@grimberg.me> Subject: Re: [dpdk-dev] [PATCH] net/mlx5: Fix possible NULL deref in RX path X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: patches and discussions about DPDK List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 02 Aug 2016 09:58:57 -0000 On Tue, Aug 02, 2016 at 12:31:35PM +0300, Sagi Grimberg wrote: > > > On 01/08/16 19:43, Adrien Mazarguil wrote: > >Hi Sagi, > > > >On Mon, Aug 01, 2016 at 11:44:21AM +0300, Sagi Grimberg wrote: > >>The user is allowed to call ->rx_pkt_burst() even without free > >>mbufs in the pool. In this scenario we'll fail allocating a rep mbuf > >>on the first iteration (where pkt is still NULL). This would cause us > >>to deref a NULL pkt (reset refcount and free). > >> > >>Fix this by checking the pkt before freeing it. > > > >Just to be sure, did you get an actual NULL deref crash here or is that an > >assumed possibility? > > > >I'm asking because this problem was supposed to be addressed by: > > > > a1bdb71a32da ("net/mlx5: fix crash in Rx") > > I actually got the NULL deref. This happens when the application doesn't > restore mbufs to the pool correctly. In the case rte_mbuf_raw_alloc > will fail on the first iteration (pkt wasn't assigned) unlike the > condition handled in a1bdb71a32da. > > With this applied, I didn't see the crash. Thanks for confirming this, now what about the different approach I suggested in my previous message to avoid the extra check in the inner loop: if (!pkt) pkt = seg; while (pkt != seg) { ... } Also the fixes line in your commit message? -- Adrien Mazarguil 6WIND