[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <878uko4b75.fsf@openvz.org>
Date: Fri, 10 Oct 2014 11:48:30 +0400
From: Dmitry Monakhov <dmonakhov@...nvz.org>
To: Andreas Dilger <adilger@...ger.ca>
Cc: linux-ext4@...r.kernel.org, sasha.levin@...cle.com
Subject: Re: [PATCH] ext4: fix race between write and fcntl(F_SETFL)
Andreas Dilger <adilger@...ger.ca> writes:
> On Oct 9, 2014, at 5:14 AM, Dmitry Monakhov <dmonakhov@...nvz.org> wrote:
>> O_DIRECT flags can be toggeled via fcntl(F_SETFL).
>> But this value checked twice inside ext4_file_write_iter() and __generic_file_write()
>> which result in BUG_ON (see typical stack trace below)
>> In order to fix this we have to use our own copy of __generic_file_write
>> and pass o_direct status explicitly.
>
> This seems like a generic problem that would be better served by fixing
> the core code instead of making a private copy of such a large function
> for ext4. I expect other filesystems might have similar issues if the
> O_DIRECT state is changed in the middle of IO?
>
> One option is to flush pending IO on the file if the O_DIRECT flag is
> changed in setfl(). This is a bit heavyweight but I can't imagine any
> sane app that is changing the O_DIRECT state on a file repeatedly.
Agree. In fact there are a lot of other issues there fcntl(F_SETFL)
works incorrectly. For example it does not works on stack-fs
(ecrypt,unionfs) if you try to add O_APPEND flags it will be directly
assigned to virtual file (not lower one). For that reason OpenVZ introduced
f_op->set_flags in order to support our stackfs (vefs)
This is reasonable solution in general so I'll prepare patches for mainstream
But this require reasonable time for negotiations with vfs people.
At the same time currently all versions are affected since 69c499d1/2012-11-29
So we need quick and simple fix for stable releases.
From first glance the best fix is to simply deny toggle O_DIRECT on files.
and f_op->check_flags(new_flags) looks like reasonable candidate, *but*
this interface is 100% useless because it has no access to file_pointer
so we do not know current ->f_flags :)
>
> Cheers, Andreas
Content of type "application/pgp-signature" skipped
Powered by blists - more mailing lists