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

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

 



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


[Index of Archives]     [Linux Kernel]     [Kernel Development Newbies]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite Hiking]     [Linux Kernel]     [Linux SCSI]