Re: [Qemu-devel] [PATCH] Fix hang with -L and symlink loop

[Peter Maydell]

On 31 May 2018 at 17:32, Richard Henderson <address@hidden> wrote:
> On 05/31/2018 08:26 AM, Peter Maydell wrote:
>> On 30 May 2018 at 21:50, Richard Henderson <address@hidden> wrote:
>>> On 05/29/2018 04:44 PM, Evan Green wrote:
>>>> When using -L with Linux app emulation, there is an issue in
>>>> init_paths where Qemu will get lost exploring a directory tree
>>>> with a symlink loop in it. This causes Qemu to hang, and
>>>> eventually consume all memory in the system.
>>>>
>>>> Qemu's code for pre-exploring the entire directory tree is both
>>>> error-prone and slow. Instead, this changes uses faccessat, which
>>>> both avoids the symlink loop (since the entire directory space isn't
>>>> being explored up front), and likely speeds things up a bit.
>>>>
>>>> Partial credit goes to Richard Henderson, as it was only after staring
>>>> at his patch [1] that I wrote mine.
>>>>
>>>> [1] https://patchwork.kernel.org/patch/9512083/
>>>>
>>>> Signed-off-by: Evan Green <address@hidden>
>>>> ---
>>>
>>> I like this as an improvement on the current situation.
>>> Perhaps folks do like this a bit better than my more invasive patch.
>>
>> It seems to me to have the same problem as your patch does,
>> which is that we now have a file descriptor which belongs
>> not to the guest but to QEMU, but we make no effort to
>> hide it from the guest, and so for instance code like that
>> xinetd "close every fd above 2" loop will break us.
>
> Yes.  I've thought about doing more explicit managing of guest fd's, but it's 
> a
> big job to re-organize that.

I think my current opinion is that it's probably OK to have the
"our internal fd is visible to the guest" problem for the case
where the user specifies -L, at least for now. We're probably
causing more problems by not allowing -L to point at trees with
symlinks than by misbehaving if the application closes it.
We should just leave ourselves a suitable TODO note or something.

As I say, I prefer your patch to this one overall, if you
wanted to address the minor issues from the last round of
review and respin it.

thanks
-- PMM