From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-wr1-f66.google.com (mail-wr1-f66.google.com [209.85.221.66]) by dpdk.org (Postfix) with ESMTP id D94625B2E for ; Wed, 24 Oct 2018 17:34:05 +0200 (CEST) Received: by mail-wr1-f66.google.com with SMTP id y16so6110697wrw.3 for ; Wed, 24 Oct 2018 08:34:05 -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:references:mime-version :content-disposition:content-transfer-encoding:in-reply-to :user-agent; bh=e3htS0DAGoXRJITfJ9pv/SqgX0ichiKmHPjLrzlz7QE=; b=gPvn3voLXQ+A+Va66OumS7zVlzozWP8j4oF1+GFMXQntBfBRAs34rWqar8yzh58zB8 d2kyXhQAcQkDm651o/md5S3dl+gMk0A8S4IaUCCpjDrB/wV1Y1Lw62dSFlnxuljndRpV dmrYN3G31ubBTTBTrtByrrXwo8U+tBWg6D+UY/FWQsryW1Z/HOQPe1+H12PBG+mIZjkG caBjbE0jhcEGhdeZFQLchAf4TZr6+gSgjMYlrYUw8Hbg8VgKLRA/FYgy+jhTu+YIUCto Cbzlb+skwa5BvV1M/AsF6WcsedvUTqSp1s729knaYhTurvGA8zkvsCHlczmhT48MW1ey 1J8A== 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:content-transfer-encoding :in-reply-to:user-agent; bh=e3htS0DAGoXRJITfJ9pv/SqgX0ichiKmHPjLrzlz7QE=; b=JBD/27Y1IwQE2Yli8C/8n2jcgKJ2JOau7uTOWtYcTdn5reKGaRWyiO2cm+j8VuYthL AHpilzxcMnVY87pw5CP2Kd2bjNjQ7YObPbBoNckAKWCaIBdpLStpDnohAbmRFcqfnT9O hEFGXTo6b6Vilipm4n7pzaYn2M5eBsTeypYEVJ4XU1hMWUYz+lu+aTEBfduyzE0yFbws Xauwe8UvvZ9r+rheBujMppHmBpuOBYmCN9tZ76LNR2/rNHu7+WwLTShFgiT2kKUOlxYY rrugh3m3tnyOaDXFSiVvA5gmUnbjsQkfWUSMH/4Gl1lj12lT/5IhOs+Lejp+WSkGQXQ1 dz9Q== X-Gm-Message-State: AGRZ1gL04BJEmATuBgVpKwXwq6oL60Vt4FwxKHoBC5GdPo3PH0Y3J59K d0A0kEHUNOtRW4JBeiTRtgkwQA== X-Google-Smtp-Source: AJdET5e9EFdyMKP46XS7OboKv5XUjojurWTtl9Y7C9HHnO/q+/OxYRkouf9YV1ONgCgCS514Xp0a1A== X-Received: by 2002:adf:9102:: with SMTP id j2-v6mr254612wrj.3.1540395244814; Wed, 24 Oct 2018 08:34:04 -0700 (PDT) Received: from bidouze.vm.6wind.com (host.78.145.23.62.rev.coltfrance.com. [62.23.145.78]) by smtp.gmail.com with ESMTPSA id 67-v6sm5980716wmd.1.2018.10.24.08.34.03 (version=TLS1_2 cipher=ECDHE-RSA-CHACHA20-POLY1305 bits=256/256); Wed, 24 Oct 2018 08:34:03 -0700 (PDT) Date: Wed, 24 Oct 2018 17:33:45 +0200 From: =?iso-8859-1?Q?Ga=EBtan?= Rivet To: Thomas Monjalon Cc: dev@dpdk.org, Qi Zhang Message-ID: <20181024153345.5sdhtitjlqqdrs6c@bidouze.vm.6wind.com> References: <20181022054932.39052-1-qi.z.zhang@intel.com> <1576298.HKmtsfqzoT@xps> <20181023223931.kmro2zfyp4c4wbqm@bidouze.vm.6wind.com> <6647495.inI2yHHxz0@xps> MIME-Version: 1.0 Content-Type: text/plain; charset=iso-8859-1 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: <6647495.inI2yHHxz0@xps> User-Agent: NeoMutt/20170113 (1.7.2) Subject: Re: [dpdk-dev] [PATCH] eal: fix floating device argument pointer X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 24 Oct 2018 15:34:06 -0000 On Wed, Oct 24, 2018 at 04:43:45PM +0200, Thomas Monjalon wrote: > 24/10/2018 00:39, Gaëtan Rivet: > > Hi, > > > > On Mon, Oct 22, 2018 at 09:25:22AM +0200, Thomas Monjalon wrote: > > > 22/10/2018 07:49, Qi Zhang: > > > > After we insert a devargs into devargs_list, following bus->scan may > > > > destroy it due to another rte_devargs_insert. Its better not to use > > > > a devargs pointer after it has been inserted. > > > > > > > A bus scan calls rte_devargs_insert? Mapping devargs to device is the > > responsibility of the bus scan, if it calls potentially destructive > > functions, it must rebuild the map. > > > > > I think the problem is in: > > > > > > rte_devargs_insert(struct rte_devargs *da) > > > { > > > int ret; > > > > > > ret = rte_devargs_remove(da); > > > if (ret < 0) > > > return ret; > > > TAILQ_INSERT_TAIL(&devargs_list, da, next); > > > return 0; > > > } > > > > > > We insert a structure which is freed! > > > > Not usually, I hope! > > > > > > > > See http://git.dpdk.org/dpdk/commit/?id=55744d83d525 > > > > > > Gaetan, what can be the fix? > > > > 1. rte_devargs_insert is misdefined. > > It is designed as a function that can never fail. > > The function should return void instead. > > > > 2. rte_devargs_remove(da), will not remove da itself. > > It will remove whichever rte_devargs matches da within the internal > > list. If da does not match any in the list, it does nothing. > > As da is a newly-callocated structure, it is actually safe to > > continue using it after having called rte_devargs_remove(), because > > it cannot possibly have been inserted in the meantime (so would not > > have been freed, even if another devargs matched it). > > If the devargs pointer passed in parameter is the same as the one > in the list, it will be freed. > This would only happen if one did: rte_devargs_insert(dev->devargs); > > The actual issue is that the matching rte_devargs within the list > > would be referenced by a device after a successful scan, meaning that > > this reference is not safe if someone attemps to insert the same > > device after the bus->scan(). If my understanding is correct, the above > > fix is not necessary, but probing should be guarded against > > re-entrancy. > > We may want to probe again with different parameters. > Sure, but in this case the fix is to check whether the device is already probed, and if so remove it before probing it again with the new devargs. > > Nice rant :) :) -- Gaëtan Rivet 6WIND