Re: [PATCH] Add dummy definition of O_CLOEXEC

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi Lucas,

On Tue, Oct 7, 2014 at 12:42 AM, Lucas De Marchi
<lucas.de.marchi@xxxxxxxxx> wrote:
>>
>> I'm trying to understand the reason you object to this patch.
>>
>> We both agree that the dummy implementation of O_CLOEXEC will only
>> take effect when building kmod using kernel headers < 2.6.23, correct?
>
> yes. But I don't want to pretend that kmod works fine there. It doesn't.
>
>> In such a case, there are three paths:
>>
>> 1. Don't make any change (the current situation). In this case, kmod
>> cannot be compiled due to the missing O_CLOEXEC definition and thus
>> cannot be used at all.
>>
>> 2. Add a dummy definition (as proposed by the patch) to allow
>> compilation of kmod. In this case, the O_CLOEXEC flag will not take
>> effect, indeed with the leaking file descriptors into child processes,
>> but only in this exceptional case where kmod is built for older
>> kernels.
>
> The problem here is that you are silently accepting the misbehave.
>
>>
>> 3. Use explicit fcntl(..., FD_CLOEXEC...) calls after the open's.
>> However, this is not easily done without impact on the standard case
>> with modern headers. I assume you will like this even less.
>>
>> To me, case 2 seems a valid solution to an otherwise unusable kmod on
>> systems with old kernels / kernel headers. The impact of the leaked
>> file descriptors is to be accepted in this case.
>
> I'm all for accepting the leaked file descriptors *in this case*. Not
> in the general case in which one takes kmod and realizes it works fine
> in < 2.6.23. Because it doesn't and pretending it does is just asking
> for invalid bug reports.
>
> So as I said, I don't think this is material for upstream.
>
> What we could do is to hide the definition of O_CLOEXEC behind a
> configure flag... but then the amount of ifdef, the benefits from it
> and having to maintain that on build sys don't pay off IMO.
>
>> However, if you still feel this is not upstreamable, then I will back
>> off and hope that Peter will accept the patch in Buildroot :)
>
> I do think this could be accepted there. AFAICS he already acked on
> it. Just make sure this is a patch that gets applied on host-kmod
> only, not the one running on target. Let's try this approach and come
> back to what I suggested above if it becomes too troublesome to
> downstream?

Thanks for your further explanation.
I will push the patch to buildroot, as suggested.

Best regards,
Thomas
--
To unsubscribe from this list: send the line "unsubscribe linux-modules" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html




[Index of Archives]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [Big List of Linux Books]

  Powered by Linux