Re: [PATCH] eCryptfs: Fix directory open regression in linux-stable

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

 



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).

> 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.

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;
>>
> 
> 


Attachment: signature.asc
Description: OpenPGP digital signature


[Index of Archives]     [Linux Crypto]     [Device Mapper Crypto]     [LARTC]     [Bugtraq]     [Yosemite Forum]

  Powered by Linux