From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-wi0-f172.google.com (mail-wi0-f172.google.com [209.85.212.172]) by dpdk.org (Postfix) with ESMTP id 7661C11C5 for ; Mon, 23 Mar 2015 12:36:46 +0100 (CET) Received: by wixw10 with SMTP id w10so59471526wix.0 for ; Mon, 23 Mar 2015 04:36:46 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=mirantis.com; s=google; h=mime-version:in-reply-to:references:date:message-id:subject:from:to :cc:content-type; bh=jtv+wmdTyawsRPQgMiN1069gs+RvXFqqlASnWGo9u8A=; b=XI0X0prjcVn/LFDvygq4PL4MAK1vxRZw5S7BN+ncBQhGEwAKqcSjtGZMpNFc1gWB6i gMs8E+PMWSU7ANHFl1LvV+JEC+vWksDlyDcb0MORTby1ma7WhyqVEDoJHfsAuloQF3Op YtWVn6Q1HYl7SPlCYoaWwL8KfB1TnkqUmbOFI= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:mime-version:in-reply-to:references:date :message-id:subject:from:to:cc:content-type; bh=jtv+wmdTyawsRPQgMiN1069gs+RvXFqqlASnWGo9u8A=; b=OTUTYAd3MZTbmE18JdlpGgU/M8rlGC/0nHPHT63NxgX2JF+AYJ1Oj0oJGO/q8lwwia ZFyBN68biV7qJ3S3b46LApqdiYodXYSE7bGc5r2Vp61GujDEPlNSZduUbcX65SYuLaZS 2VX+Ai3F8JKCzxqndBXAfA5hvq3d4ZQTIsvP58FWIOGIs9K0yDo5B+cQLij2wGh2tEuw m5gzoCmkiQ7IcvMNJcl078zHiQkEBMODlY+pg0PqOu+Z4dCW6QdNcEyKm1+BOLHQJB+v 34Id0vSCcH5yJ6LjRTD9MHTvyQFu3XTh+p0bHnKiz53OJ1vq6rbTfuCWfzMPo9Jvh7sM r1tA== X-Gm-Message-State: ALoCoQlMWcZaplX5sGXuGsBQ8QRmU7GFv5pO1JPOG560U+mcQBgnlQvhncsldKk0QYI6fChiXkQu MIME-Version: 1.0 X-Received: by 10.194.91.129 with SMTP id ce1mr141870274wjb.53.1427110606280; Mon, 23 Mar 2015 04:36:46 -0700 (PDT) Received: by 10.194.76.7 with HTTP; Mon, 23 Mar 2015 04:36:46 -0700 (PDT) In-Reply-To: <2544479.OZtlEpgWBm@xps13> References: <1426524059-30886-1-git-send-email-pboldin+dpdk@mirantis.com> <1426684571-14782-1-git-send-email-pboldin@mirantis.com> <2544479.OZtlEpgWBm@xps13> Date: Mon, 23 Mar 2015 13:36:46 +0200 Message-ID: From: Pavel Boldin To: Thomas Monjalon Content-Type: text/plain; charset=UTF-8 X-Content-Filtered-By: Mailman/MimeDel 2.1.15 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:36:46 -0000 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? Pavel 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 > > > >