Re: [dpdk-dev] [PATCH v2] Fix `eventfd_link' module leakages and races

[Thomas Monjalon <thomas.monjalon@6wind.com>]

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 <thomas.monjalon@6wind.com>
> 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