On 7/7/16 2:02 PM, Tyler Hicks wrote:
> On 07/05/2016 04:14 PM, Jeff Mahoney wrote:
>> On 6/28/16 11:39 PM, Tyler Hicks wrote:
>>> Cherry-picking mainline commit 2f36db71009304b3f0b95afacd8eba1f9f046b87
>>> introduces a regression in eCryptfs when mainline commit
>>> 6a480a7842545ec520a91730209ec0bae41694c1 (4.6+) is not present. The
>>> regression causes all attempts at opening directory files to fail with
>>> EMEDIUMTYPE when the lower filesystem's file_operations for directory
>>> files do not implement mmap.
>>>
>>> This is a simple fix that allows the check for the lower file's mmap
>>> implementation to be ignored if the lower file is a directory.
>>
>> I have a different fix that I believe is more correct for this. I would
>> have posted it in response to the original fix if it were ever actually
>> posted for public discussion.
>
> Hi Jeff - I'm glad that you are sending your fix out for review (not
> sure if you noticed but I asked you to do so in a reply to your comment
> on the project zero blog post).
I didn't see that. Strange that it linked my profile pic but didn't
send a notification. :)
>> Denying open is the wrong place to fix this. It's too heavy a hammer
>> and, as we see here, a bit fragile.
>
> I agree but I think that denying open() has little to do with this
> regression in linux-stable. I'd prefer that we leave linux-stable as-is
> (after applying my minimal regression fix patch) and take your fix trunk.
It doesn't have anything to do with the regression -- but it can be
backported directly to stable without regressions if the original fix is
removed and stable fixes should match the upstream ones.
-Jeff
> Tyler
>
>>
>> The right fix is to deny the mmap call instead.
>>
>> -Jeff
>>
>>> Signed-off-by: Tyler Hicks <tyhicks@xxxxxxxxxxxxx>
>>> Tested-by: Tyler Hicks <tyhicks@xxxxxxxxxxxxx> # 4.4.y, 3.18.y
>>> Cc: <stable@xxxxxxxxxxxxxxx> # 4.5-
>>> ---
>>> fs/ecryptfs/kthread.c | 2 +-
>>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>>
>>> diff --git a/fs/ecryptfs/kthread.c b/fs/ecryptfs/kthread.c
>>> index e818f5a..b9faeab 100644
>>> --- a/fs/ecryptfs/kthread.c
>>> +++ b/fs/ecryptfs/kthread.c
>>> @@ -171,7 +171,7 @@ int ecryptfs_privileged_open(struct file **lower_file,
>>> goto out;
>>> }
>>> have_file:
>>> - if ((*lower_file)->f_op->mmap == NULL) {
>>> + if ((*lower_file)->f_op->mmap == NULL && !d_is_dir(lower_dentry)) {
>>> fput(*lower_file);
>>> *lower_file = NULL;
>>> rc = -EMEDIUMTYPE;
>>>
>>
>>
>
>
--
Jeff Mahoney
SUSE Labs
Attachment:
signature.asc
Description: OpenPGP digital signature
