From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-wi0-f175.google.com (mail-wi0-f175.google.com [209.85.212.175]) by dpdk.org (Postfix) with ESMTP id A0A1D11C5 for ; Mon, 23 Mar 2015 12:44:44 +0100 (CET) Received: by wibgn9 with SMTP id gn9so59672625wib.1 for ; Mon, 23 Mar 2015 04:44:44 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:from:to:cc:subject:date:message-id:organization :user-agent:in-reply-to:references:mime-version :content-transfer-encoding:content-type; bh=cdxxg1rI3cp1jE/pOARi8Ulvq3w3v7ZdIQL0VeJ3h0A=; b=FB7yrBQTkPEbe411xgmFW8W/sX6v62m36rFH/hzSGPEQ7ofVh6/SUX8hU63h2Hd6xE RiwBLiU21lU2gWDOnz2ggljSUQILLs9d5mjKqd1G7phhWF9WYBAZLX5NHCwb61iWsEV2 aVCPBghUCFOoAsg6Ih8EibXG2g/1Ckb6NXoUe8yb3lZNMBXC2mtrJDPL189qfaVw2XU7 hBmH7121r7ev3pJ8LKzjpKdbN7FyAPGpUi4V2LQ3D4+4Vwvq8urPv/4NZ7NVlgPgLT/W z4sYirg9pb26/6Dw10eeMsyCibjcLISmKOwI7QY6mukGZgtKo4n68LdeeFlRhAouqSAN ApNQ== X-Gm-Message-State: ALoCoQlHeDyldYaKTm2AowLbpHxOtgTfjKa8ZcLSYJ3ZH8oTV/HXVgkK58DIg0SYWhACPz2NoIAu X-Received: by 10.195.12.167 with SMTP id er7mr187691257wjd.54.1427111084488; Mon, 23 Mar 2015 04:44:44 -0700 (PDT) Received: from xps13.localnet (136-92-190-109.dsl.ovh.fr. [109.190.92.136]) by mx.google.com with ESMTPSA id ga8sm10763205wib.11.2015.03.23.04.44.43 (version=TLSv1.2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Mon, 23 Mar 2015 04:44:43 -0700 (PDT) From: Thomas Monjalon To: Pavel Boldin Date: Mon, 23 Mar 2015 12:44:03 +0100 Message-ID: <2367297.OP1GXNRJq9@xps13> Organization: 6WIND User-Agent: KMail/4.14.4 (Linux/3.18.4-1-ARCH; KDE/4.14.4; x86_64; ; ) In-Reply-To: References: <1426524059-30886-1-git-send-email-pboldin+dpdk@mirantis.com> <2544479.OZtlEpgWBm@xps13> MIME-Version: 1.0 Content-Transfer-Encoding: 7Bit Content-Type: text/plain; charset="us-ascii" Cc: dev@dpdk.org Subject: Re: [dpdk-dev] [PATCH v2] Fix `eventfd_link' module leakages and races 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: Mon, 23 Mar 2015 11:44:44 -0000 2015-03-23 13:36, Pavel Boldin: > Hi Thomas, > > There is by far more than 2 issues, but these are the only ones that > appeared to us. > > The list of the issues that motivated the refactoring: > > 1. Only one error code for every fault (-EFAULT). > 2. Copy-paste code from the `fget' function. > 3. Ambiguous variable names. The `files' variable mean different things > under the same block. This ruins readability of the code. > 4. Overall placing of the all the code inside one `switch/case'. Modern > kernel code style suggests placing these in the inline functions. > 5. Usage of the copy-pasted `close_fd' function code. (I really should > use `sys_close' here though). > > > I can rewrite the refactoring in a set of small patches if this will aid > reviewers. > > The dangerous bug that definitely must be fixed before the next release is > the `struct file*' leakage. I can provide a simple patch for this as a > separate submission and continue working on the refactoring patches. Is > this plan OK for you? Yes, please separate important fixes and refactoring. Then we'll be able to merge the fixes in 2.0. PS: please answer below the question to ease thread reading. Thanks > On Mon, Mar 23, 2015 at 1:15 PM, Thomas Monjalon > wrote: > > > Hi Pavel, > > > > You forgot the Signed-off-by line. > > > > Huawei, Changchun, any comment on this patch? > > > > 2015-03-18 15:16, Pavel Boldin: > > > The `eventfd_link' module provides an API to "steal" fd from another > > > process had been written with a bug that leaks `struct file' because > > > of the extra reference counter increment and missing `fput' call. > > > > > > The other bug is using another process' `task_struct' without > > incrementing > > > a reference counter. > > > > As there are 2 bugs, you should provide 2 patches. > > > > > Fix these bugs and refactor the module. > > > > Why refactoring along with the bug fixes? > > You'd have more chances to have your fixes integrated in 2.0 without > > refactoring. > > > > Thanks