* RE: ecryptfs and fanotify
[not found] ` <CANT6BaPhaAOv3hg9CgoDa9JTp9vN60zf8bdAQX+dFp6YLSUwnA@mail.gmail.com>
@ 2012-08-14 9:00 ` Douglas Leeder
[not found] ` <F85176E33E12BA45BD553324E7858B300CE0A5@abn-exch1b.green.sophos>
1 sibling, 0 replies; 7+ messages in thread
From: Douglas Leeder @ 2012-08-14 9:00 UTC (permalink / raw)
To: Dustin Kirkland, Tyler C Hicks
Cc: Eric Paris, malware-list@dmesg.printk.net, Eddie Garcia,
Sergio Pena, Florian Westphal, ecryptfs@vger.kernel.org
On 13/08/12 18:17, Dustin Kirkland wrote:
> Howdy Douglas,
>
> Responses are inline below...
>
> On Wed, Aug 8, 2012 at 7:57 AM, Douglas Leeder
> <douglas.leeder@sophos.com> wrote:
>> Hi Dustin, Eric;
>>
>> We are in the final stages of producing a version of Sophos
>> Anti-Virus for Linux that uses fanotify, and have hit an issue with
>> fanotify against ecryptfs mounts.
>
> Ah, interesting. Okay, so I've worked with Sophos in the past, to
> add eCryptfs to the do-not-scan-at-mount list for talpa. I'm not
> familiar with fanotify -- is it a replacement for what Sophos was
> previously using talpa to do (watch filesystem events)?
Yes, fanotify is the built-in kernel code, that allows user space to
monitor file events for entire filesystems, similar to inotify/dnotify
except for entire filesystems.
http://lwn.net/Articles/339253/
It is an alternative to Talpa, for newer kernels.
>
>> Eric is the person who did the vast majority of the work in getting
>> fanotify into the kernel. Dustin is the upstream developer for
>> ecryptfs, as well as an Ubuntu core developer.
>>
>> (At least that was the case at my last contact - if there are more
>> appropriate people to contact then please forward this on).
>
> Right, I am the eCryptfs userspace maintainer. I have CC'd Tyler
> Hicks, who is the eCryptfs kernel module maintainer.
>
>> ecryptfs is used for encrypted home directories on (at least)
>> Ubuntu.
>
> That's one place. eCryptfs is also used in Debian, Arch, Fedora,
> CentOS, RHEL, Google's ChromeOS, Seagate's NAS, Motorola Android
> phones, and Gazzang's encryption products.
Indeed, that is why I added the "(at least)" :-)
>
>> This issue I'm seeing is on Ubuntu 12.04 with encrypted home
>> directories. (3.2 kernel)
>>
>> After using Firefox for a short while with fanotify scanning
>> happening (for both the host fs, and the ecryptfs mount), it will
>> go grey (or gray :-)), signalling it has hung.
>>
>> Two threads in Firefox with be in the D state (IO sleep), and two
>> of the scanner threads will be stuck reading from the fanotify fd.
>> These remain stuck until the machine is rebooted.
>>
>> I've got a test 'scanner' that I've managed to reproduce the issue
>> with:
>>
>> Attached tarfile: fanotify.ecrypyfs.tgz
>>
>> github: https://github.com/paperclip/fanotify-ecryptfs-test
>>
>>
>> However it doesn't happen every time, and I've not reproduced it on
>> a VM.
>>
>> Points on note: 1) Every file on ecryptfs is backed by a file on
>> the host fs 2) Thus every access to an ecryptfs also generates a
>> file access to the host fs. 3) fanotify can deliver multiple file
>> accesses in a single event.
>
> What do you want to scan? The lower, encrypted files, or the upper,
> decrypted (cleartext) ones?
So the problem is that fanotify works on a filesystem level, so
we scan the ecryptfs to actually scan the clear version of the files, and get the encrypted version as a side-effect of scanning the root/parent filesystem. (Since ecryptfs encrypted files are normally stored on the normal parent fs)
We can't usefully scan the encrypted files, but can't exclude them.
>
>> Given the unreliable nature of the reproduction steps; and the fact
>> that when things hang it is always 2 threads, rather than 1 or 3
>> that hang: I believe that there is some kind of race condition when
>> more than one file on an ecryptfs mount is accessed at the same
>> time.
>
> Tyler, you've looked a few race conditions recently... Does this
> one ring a bell?
I think it must be a race condition, at least in part, since it doesn't happen
every time, and at a different point each time, and simple file access is fine.
>
>> I wonder if each fanotify event contains the ecryptfs access from
>> one client thread, and the host fs access from the other?
>>
>> Is there any advice you can offer? I wonder if fanotify needs to
>> deliver permission events one at a time? Or can you point me at
>> appropriate sources to look at for further information.
>>
>> In an ideal world there would be some fix to the scanner to avoid
>> the problem, but I'm not sure what it could be, so the solution
>> might only be applicable to future kernels.
>
> Interesting. Perhaps an I/O synchronicity problem. I don't know
> much about Sophos or fanotify, but at a high level, I would think
> that Sophos would need to watch the upper, clear text files right?
> Those clear text files, of course, don't actually exist as clear text
> files. They're decrypted and rendered in memory on demand. Since
> eCryptfs is a layered filesystem, perhaps fanotify might need to
> handle things a little differently, watching events only on the lower
> filesystem?
>
So as, I sent later, making fanotify only return one file event per fanotify read(), So that suggests that the request is getting stuck in a cycle or adding a request.
Florian has also been looking at this problem, and has pointed out that FMODE_NONOTIFY should prevent the loop seen in the backtrace, so I'm wondering if it's getting lost somewhere in ecryptfs. Or perhaps it needs to be explicitly propagated to the open request for the lower file?
Thanks.
--
Douglas Leeder, Senior Software Engineer
________________________________
Sophos Limited, The Pentagon, Abingdon Science Park, Abingdon, OX14 3YP, United Kingdom.
Company Reg No 2096520. VAT Reg No GB 991 2418 08.
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: ecryptfs and fanotify
[not found] ` <F85176E33E12BA45BD553324E7858B300CE0A5-wyMY/JD5BThOaAOxIYwbkpbnZX9N9W2w@public.gmane.org>
@ 2012-08-14 9:31 ` Florian Westphal
0 siblings, 0 replies; 7+ messages in thread
From: Florian Westphal @ 2012-08-14 9:31 UTC (permalink / raw)
To: Douglas Leeder
Cc: Sergio Pena, malware-list-h+Im9A44IAFcMpApZELgcQ@public.gmane.org,
ecryptfs-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, Eric Paris,
Tyler C Hicks, Dustin Kirkland, Eddie Garcia
Douglas Leeder <douglas.leeder-j34lQMj1tz/QT0dZR+AlfA@public.gmane.org> wrote:
> So as, I sent later, making fanotify only return one file event per fanotify read(), So that suggests that the request is getting stuck in a cycle or adding a request.
>
> Florian has also been looking at this problem, and has pointed out that FMODE_NONOTIFY should prevent the loop seen in the backtrace, so I'm wondering if it's getting lost somewhere in ecryptfs. Or perhaps it needs to be explicitly propagated to the open request for the lower file?
Douglas, can you reproduce the problem with this patch applied?
diff --git a/fs/ecryptfs/kthread.c b/fs/ecryptfs/kthread.c
index 809e67d..1d8a53b 100644
--- a/fs/ecryptfs/kthread.c
+++ b/fs/ecryptfs/kthread.c
@@ -74,7 +74,7 @@ static int ecryptfs_threadfn(void *ignored)
kthread_ctl_list);
list_del(&req->kthread_ctl_list);
*req->lower_file = dentry_open(&req->path,
- (O_RDWR | O_LARGEFILE), current_cred());
+ (O_RDWR | O_LARGEFILE | FMODE_NONOTIFY), current_cred());
complete(&req->done);
}
mutex_unlock(&ecryptfs_kthread_ctl.mux);
@@ -133,7 +133,7 @@ int ecryptfs_privileged_open(struct file **lower_file,
const struct cred *cred)
{
struct ecryptfs_open_req req;
- int flags = O_LARGEFILE;
+ int flags = O_LARGEFILE|FMODE_NONOTIFY;
int rc = 0;
init_completion(&req.done);
^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: ecryptfs and fanotify
[not found] ` <20120814093124.GA12898-OOs/4mkCeqbQT0dZR+AlfA@public.gmane.org>
@ 2012-08-14 10:50 ` Douglas Leeder
0 siblings, 0 replies; 7+ messages in thread
From: Douglas Leeder @ 2012-08-14 10:50 UTC (permalink / raw)
To: Florian Westphal
Cc: Sergio Pena, malware-list-h+Im9A44IAFcMpApZELgcQ@public.gmane.org,
ecryptfs-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, Eric Paris,
Tyler C Hicks, Dustin Kirkland, Eddie Garcia
On 14/08/12 10:31, Florian Westphal wrote:
> Douglas Leeder <douglas.leeder-j34lQMj1tz/QT0dZR+AlfA@public.gmane.org> wrote:
>> So as, I sent later, making fanotify only return one file event per fanotify read(), So that suggests that the request is getting stuck in a cycle or adding a request.
>>
>> Florian has also been looking at this problem, and has pointed out that FMODE_NONOTIFY should prevent the loop seen in the backtrace, so I'm wondering if it's getting lost somewhere in ecryptfs. Or perhaps it needs to be explicitly propagated to the open request for the lower file?
>
> Douglas, can you reproduce the problem with this patch applied?
>
> diff --git a/fs/ecryptfs/kthread.c b/fs/ecryptfs/kthread.c
> index 809e67d..1d8a53b 100644
> --- a/fs/ecryptfs/kthread.c
> +++ b/fs/ecryptfs/kthread.c
> @@ -74,7 +74,7 @@ static int ecryptfs_threadfn(void *ignored)
> kthread_ctl_list);
> list_del(&req->kthread_ctl_list);
> *req->lower_file = dentry_open(&req->path,
> - (O_RDWR | O_LARGEFILE), current_cred());
> + (O_RDWR | O_LARGEFILE | FMODE_NONOTIFY), current_cred());
> complete(&req->done);
> }
> mutex_unlock(&ecryptfs_kthread_ctl.mux);
> @@ -133,7 +133,7 @@ int ecryptfs_privileged_open(struct file **lower_file,
> const struct cred *cred)
> {
> struct ecryptfs_open_req req;
> - int flags = O_LARGEFILE;
> + int flags = O_LARGEFILE|FMODE_NONOTIFY;
> int rc = 0;
>
> init_completion(&req.done);
>
That patch also seems to fix the problem - at least for AV scanning:
1) The encrypted files don't show up in the user-space test program.
2) Using the FanotifyMultiThreadScanner (old) from the git tree doesn't
lock up. (With the single event read() patch removed for the moment).
However I don't think this is a sufficient solution for general fanotify.
For example an HSM may need to get the fanotify event to restore the
encrypted file to disk.
Or a file-access recorder (e.g. http://www.piware.de/tag/fanotify/ )
will want all the accesses, including those 'generated' by the
fanotify_read() call itself.
Talpa, in this case, has it easier, since it is only used for AV, it can
ignore any accesses that it's caused.
What we might be able to do is set FMODE_NONOTIFY for the lower access
if the upper access had it set? Since HSM and fatrace don't want to
access the contents when they intercept.
I guess HSM and fatrace might not intercept ecryptfs mounts anyway,
since they don't directly access the disk?
--
Douglas Leeder, Senior Software Engineer
________________________________
Sophos Limited, The Pentagon, Abingdon Science Park, Abingdon, OX14 3YP, United Kingdom.
Company Reg No 2096520. VAT Reg No GB 991 2418 08.
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: ecryptfs and fanotify
[not found] ` <502A2D77.50805@sophos.com>
@ 2012-08-14 16:01 ` Douglas Leeder
2012-08-14 20:27 ` Tyler C Hicks
0 siblings, 1 reply; 7+ messages in thread
From: Douglas Leeder @ 2012-08-14 16:01 UTC (permalink / raw)
To: Florian Westphal
Cc: Dustin Kirkland, Tyler C Hicks, Eric Paris,
malware-list@dmesg.printk.net, Eddie Garcia, Sergio Pena,
ecryptfs@vger.kernel.org
On 14/08/12 11:50, Douglas Leeder wrote:
> On 14/08/12 10:31, Florian Westphal wrote:
>> Douglas Leeder <douglas.leeder@sophos.com> wrote:
>>> So as, I sent later, making fanotify only return one file event per
>>> fanotify read(), So that suggests that the request is getting stuck
>>> in a cycle or adding a request.
>>>
>>> Florian has also been looking at this problem, and has pointed out
>>> that FMODE_NONOTIFY should prevent the loop seen in the backtrace, so
>>> I'm wondering if it's getting lost somewhere in ecryptfs. Or perhaps
>>> it needs to be explicitly propagated to the open request for the
>>> lower file?
>>
>> Douglas, can you reproduce the problem with this patch applied?
>>
>> diff --git a/fs/ecryptfs/kthread.c b/fs/ecryptfs/kthread.c
>> index 809e67d..1d8a53b 100644
>> --- a/fs/ecryptfs/kthread.c
>> +++ b/fs/ecryptfs/kthread.c
>> @@ -74,7 +74,7 @@ static int ecryptfs_threadfn(void *ignored)
>> kthread_ctl_list);
>> list_del(&req->kthread_ctl_list);
>> *req->lower_file = dentry_open(&req->path,
>> - (O_RDWR | O_LARGEFILE), current_cred());
>> + (O_RDWR | O_LARGEFILE | FMODE_NONOTIFY),
>> current_cred());
>> complete(&req->done);
>> }
>> mutex_unlock(&ecryptfs_kthread_ctl.mux);
>> @@ -133,7 +133,7 @@ int ecryptfs_privileged_open(struct file
>> **lower_file,
>> const struct cred *cred)
>> {
>> struct ecryptfs_open_req req;
>> - int flags = O_LARGEFILE;
>> + int flags = O_LARGEFILE|FMODE_NONOTIFY;
>> int rc = 0;
>>
>> init_completion(&req.done);
>>
>
> That patch also seems to fix the problem - at least for AV scanning:
> 1) The encrypted files don't show up in the user-space test program.
> 2) Using the FanotifyMultiThreadScanner (old) from the git tree doesn't
> lock up. (With the single event read() patch removed for the moment).
>
>
> However I don't think this is a sufficient solution for general fanotify.
> For example an HSM may need to get the fanotify event to restore the
> encrypted file to disk.
> Or a file-access recorder (e.g. http://www.piware.de/tag/fanotify/ )
> will want all the accesses, including those 'generated' by the
> fanotify_read() call itself.
>
> Talpa, in this case, has it easier, since it is only used for AV, it can
> ignore any accesses that it's caused.
>
> What we might be able to do is set FMODE_NONOTIFY for the lower access
> if the upper access had it set? Since HSM and fatrace don't want to
> access the contents when they intercept.
>
> I guess HSM and fatrace might not intercept ecryptfs mounts anyway,
> since they don't directly access the disk?
>
So I've looked at the path required for that and I guess it involves
passing file->f_flags from ecryptfs_open through:
* ecryptfs_get_lower_file (main.c)
* ecryptfs_init_lower_file (main.c)
* ecryptfs_privileged_open (kthread.c)
Does that look like the correct to you, Tyler?
I think HSM and fatrace usages won't request the file be opened, so
won't trigger that path though fanotify.
--
Douglas Leeder, Senior Software Engineer
________________________________
Sophos Limited, The Pentagon, Abingdon Science Park, Abingdon, OX14 3YP, United Kingdom.
Company Reg No 2096520. VAT Reg No GB 991 2418 08.
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: ecryptfs and fanotify
2012-08-14 16:01 ` Douglas Leeder
@ 2012-08-14 20:27 ` Tyler C Hicks
2012-08-20 8:46 ` Douglas Leeder
2012-08-20 14:53 ` Douglas Leeder
0 siblings, 2 replies; 7+ messages in thread
From: Tyler C Hicks @ 2012-08-14 20:27 UTC (permalink / raw)
To: Douglas Leeder
Cc: Florian Westphal, Dustin Kirkland, Eric Paris,
malware-list@dmesg.printk.net, Eddie Garcia, Sergio Pena,
ecryptfs@vger.kernel.org
[-- Attachment #1: Type: text/plain, Size: 4142 bytes --]
On 2012-08-14 17:01:14, Douglas Leeder wrote:
> On 14/08/12 11:50, Douglas Leeder wrote:
> >On 14/08/12 10:31, Florian Westphal wrote:
> >>Douglas Leeder <douglas.leeder@sophos.com> wrote:
> >>>So as, I sent later, making fanotify only return one file event per
> >>>fanotify read(), So that suggests that the request is getting stuck
> >>>in a cycle or adding a request.
> >>>
> >>>Florian has also been looking at this problem, and has pointed out
> >>>that FMODE_NONOTIFY should prevent the loop seen in the backtrace, so
> >>>I'm wondering if it's getting lost somewhere in ecryptfs. Or perhaps
> >>>it needs to be explicitly propagated to the open request for the
> >>>lower file?
> >>
> >>Douglas, can you reproduce the problem with this patch applied?
> >>
> >>diff --git a/fs/ecryptfs/kthread.c b/fs/ecryptfs/kthread.c
> >>index 809e67d..1d8a53b 100644
> >>--- a/fs/ecryptfs/kthread.c
> >>+++ b/fs/ecryptfs/kthread.c
> >>@@ -74,7 +74,7 @@ static int ecryptfs_threadfn(void *ignored)
> >> kthread_ctl_list);
> >> list_del(&req->kthread_ctl_list);
> >> *req->lower_file = dentry_open(&req->path,
> >>- (O_RDWR | O_LARGEFILE), current_cred());
> >>+ (O_RDWR | O_LARGEFILE | FMODE_NONOTIFY),
> >>current_cred());
> >> complete(&req->done);
> >> }
> >> mutex_unlock(&ecryptfs_kthread_ctl.mux);
> >>@@ -133,7 +133,7 @@ int ecryptfs_privileged_open(struct file
> >>**lower_file,
> >> const struct cred *cred)
> >> {
> >> struct ecryptfs_open_req req;
> >>- int flags = O_LARGEFILE;
> >>+ int flags = O_LARGEFILE|FMODE_NONOTIFY;
> >> int rc = 0;
> >>
> >> init_completion(&req.done);
> >>
> >
> >That patch also seems to fix the problem - at least for AV scanning:
> >1) The encrypted files don't show up in the user-space test program.
> >2) Using the FanotifyMultiThreadScanner (old) from the git tree doesn't
> >lock up. (With the single event read() patch removed for the moment).
> >
> >
> >However I don't think this is a sufficient solution for general fanotify.
> >For example an HSM may need to get the fanotify event to restore the
> >encrypted file to disk.
> >Or a file-access recorder (e.g. http://www.piware.de/tag/fanotify/ )
> >will want all the accesses, including those 'generated' by the
> >fanotify_read() call itself.
> >
> >Talpa, in this case, has it easier, since it is only used for AV, it can
> >ignore any accesses that it's caused.
> >
> >What we might be able to do is set FMODE_NONOTIFY for the lower access
> >if the upper access had it set? Since HSM and fatrace don't want to
> >access the contents when they intercept.
> >
> >I guess HSM and fatrace might not intercept ecryptfs mounts anyway,
> >since they don't directly access the disk?
> >
>
> So I've looked at the path required for that and I guess it involves
> passing file->f_flags from ecryptfs_open through:
> * ecryptfs_get_lower_file (main.c)
> * ecryptfs_init_lower_file (main.c)
> * ecryptfs_privileged_open (kthread.c)
>
> Does that look like the correct to you, Tyler?
Yeah, that should be the full list.
However, I don't think this approach will work with the current
eCryptfs design. eCryptfs only keeps one lower file open per inode. That
means that there could be 10 file descriptors opened for a given
eCryptfs file but there is still only one lower file opened that
everything is multiplexed through.
I don't like the design, but it has been that way for years and I
haven't touched it. For your approach to work, I think eCryptfs would
need to be changed to have a 1-to-1 mapping of upper files to lower
files.
Tyler
>
> I think HSM and fatrace usages won't request the file be opened, so
> won't trigger that path though fanotify.
>
> --
> Douglas Leeder, Senior Software Engineer
>
> ________________________________
>
> Sophos Limited, The Pentagon, Abingdon Science Park, Abingdon, OX14 3YP, United Kingdom.
> Company Reg No 2096520. VAT Reg No GB 991 2418 08.
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: ecryptfs and fanotify
2012-08-14 20:27 ` Tyler C Hicks
@ 2012-08-20 8:46 ` Douglas Leeder
2012-08-20 14:53 ` Douglas Leeder
1 sibling, 0 replies; 7+ messages in thread
From: Douglas Leeder @ 2012-08-20 8:46 UTC (permalink / raw)
To: Tyler C Hicks
Cc: Florian Westphal,
malware-list-h+Im9A44IAFcMpApZELgcQ@public.gmane.org, Sergio Pena,
ecryptfs-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, Eric Paris,
Dustin Kirkland, Eddie Garcia
On 14/08/12 21:27, Tyler C Hicks wrote:
>>
>> So I've looked at the path required for that and I guess it involves
>> passing file->f_flags from ecryptfs_open through:
>> * ecryptfs_get_lower_file (main.c)
>> * ecryptfs_init_lower_file (main.c)
>> * ecryptfs_privileged_open (kthread.c)
>>
>> Does that look like the correct to you, Tyler?
>
> Yeah, that should be the full list.
>
> However, I don't think this approach will work with the current
> eCryptfs design. eCryptfs only keeps one lower file open per inode. That
> means that there could be 10 file descriptors opened for a given
> eCryptfs file but there is still only one lower file opened that
> everything is multiplexed through.
I don't think that matters, either:
a) The file is already open, in which case the fanotify request will
simply pass on the already opened lower file. Or
b) The file is not open, and the flag will be passed onto the new lower
file open request.
For HSM: The lower file will be restored when the first open happens,
which is as required. Presumably the file won't be closed until the last
upper file descriptor is closed, so the HSM won't archive the file too
early either.
For fatrace: The initial open is recorded, and then writes? (I haven't
looked at fatrace in detail).
For AV: The lower file can't be scanned anyway, so it doesn't matter as
long as it doesn't lock up the system.
--
Douglas Leeder, Senior Software Engineer
________________________________
Sophos Limited, The Pentagon, Abingdon Science Park, Abingdon, OX14 3YP, United Kingdom.
Company Reg No 2096520. VAT Reg No GB 991 2418 08.
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: ecryptfs and fanotify
2012-08-14 20:27 ` Tyler C Hicks
2012-08-20 8:46 ` Douglas Leeder
@ 2012-08-20 14:53 ` Douglas Leeder
1 sibling, 0 replies; 7+ messages in thread
From: Douglas Leeder @ 2012-08-20 14:53 UTC (permalink / raw)
To: Tyler C Hicks
Cc: Florian Westphal, Dustin Kirkland, Eric Paris,
malware-list@dmesg.printk.net, Eddie Garcia, Sergio Pena,
ecryptfs@vger.kernel.org
[-- Attachment #1: Type: text/plain, Size: 1465 bytes --]
On 14/08/12 21:27, Tyler C Hicks wrote:
> On 2012-08-14 17:01:14, Douglas Leeder wrote:
>> So I've looked at the path required for that and I guess it involves
>> passing file->f_flags from ecryptfs_open through:
>> * ecryptfs_get_lower_file (main.c)
>> * ecryptfs_init_lower_file (main.c)
>> * ecryptfs_privileged_open (kthread.c)
>>
>> Does that look like the correct to you, Tyler?
>
> Yeah, that should be the full list.
>
> However, I don't think this approach will work with the current
> eCryptfs design. eCryptfs only keeps one lower file open per inode. That
> means that there could be 10 file descriptors opened for a given
> eCryptfs file but there is still only one lower file opened that
> everything is multiplexed through.
>
> I don't like the design, but it has been that way for years and I
> haven't touched it. For your approach to work, I think eCryptfs would
> need to be changed to have a 1-to-1 mapping of upper files to lower
> files.
>
I've attached my patch to pass the flags though, and apply the
FMODE_NONOTIFY flag to the lower access.
The other callers of ecryptfs_get_lower_file just pass in zero, so
will continue with the existing behaviour.
What do you think?
--
Douglas Leeder, Senior Software Engineer
________________________________
Sophos Limited, The Pentagon, Abingdon Science Park, Abingdon, OX14 3YP, United Kingdom.
Company Reg No 2096520. VAT Reg No GB 991 2418 08.
[-- Attachment #2: 0001-Pass-the-f_flags-from-the-upper-open-to-the-lower-op.patch --]
[-- Type: text/x-patch, Size: 6323 bytes --]
From 8d204f86caefbbbe918ecd36ff1dfeacaa71eefd Mon Sep 17 00:00:00 2001
From: Douglas Leeder <douglas.leeder@sophos.com>
Date: Mon, 20 Aug 2012 15:33:29 +0100
Subject: [PATCH] Pass the f_flags from the upper open to the lower open.
Ensure that if the upper open happens with FMODE_NONOTIFY set,
then the lower open also does that.
This should prevent the dead-lock with fanotify and ecryptfs.
---
fs/ecryptfs/ecryptfs_kernel.h | 5 +++--
fs/ecryptfs/file.c | 2 +-
fs/ecryptfs/inode.c | 8 ++++----
fs/ecryptfs/kthread.c | 6 +++++-
fs/ecryptfs/main.c | 8 ++++----
5 files changed, 17 insertions(+), 12 deletions(-)
diff --git a/fs/ecryptfs/ecryptfs_kernel.h b/fs/ecryptfs/ecryptfs_kernel.h
index cfb4b9f..651477a 100644
--- a/fs/ecryptfs/ecryptfs_kernel.h
+++ b/fs/ecryptfs/ecryptfs_kernel.h
@@ -668,8 +668,9 @@ void ecryptfs_destroy_kthread(void);
int ecryptfs_privileged_open(struct file **lower_file,
struct dentry *lower_dentry,
struct vfsmount *lower_mnt,
- const struct cred *cred);
-int ecryptfs_get_lower_file(struct dentry *dentry, struct inode *inode);
+ const struct cred *cred,
+ unsigned int upper_file_flags);
+int ecryptfs_get_lower_file(struct dentry *dentry, struct inode *inode, unsigned int upper_file_flags);
void ecryptfs_put_lower_file(struct inode *inode);
int
ecryptfs_write_tag_70_packet(char *dest, size_t *remaining_bytes,
diff --git a/fs/ecryptfs/file.c b/fs/ecryptfs/file.c
index 44ce5c6..aa8aad5 100644
--- a/fs/ecryptfs/file.c
+++ b/fs/ecryptfs/file.c
@@ -232,7 +232,7 @@ static int ecryptfs_open(struct inode *inode, struct file *file)
| ECRYPTFS_ENCRYPTED);
}
mutex_unlock(&crypt_stat->cs_mutex);
- rc = ecryptfs_get_lower_file(ecryptfs_dentry, inode);
+ rc = ecryptfs_get_lower_file(ecryptfs_dentry, inode, file->f_flags);
if (rc) {
printk(KERN_ERR "%s: Error attempting to initialize "
"the lower file for the dentry with name "
diff --git a/fs/ecryptfs/inode.c b/fs/ecryptfs/inode.c
index 534b129..847ced2 100644
--- a/fs/ecryptfs/inode.c
+++ b/fs/ecryptfs/inode.c
@@ -246,7 +246,7 @@ int ecryptfs_initialize_file(struct dentry *ecryptfs_dentry,
"context; rc = [%d]\n", rc);
goto out;
}
- rc = ecryptfs_get_lower_file(ecryptfs_dentry, ecryptfs_inode);
+ rc = ecryptfs_get_lower_file(ecryptfs_dentry, ecryptfs_inode, 0);
if (rc) {
printk(KERN_ERR "%s: Error attempting to initialize "
"the lower file for the dentry with name "
@@ -309,7 +309,7 @@ static int ecryptfs_i_size_read(struct dentry *dentry, struct inode *inode)
struct ecryptfs_crypt_stat *crypt_stat;
int rc;
- rc = ecryptfs_get_lower_file(dentry, inode);
+ rc = ecryptfs_get_lower_file(dentry, inode, 0);
if (rc) {
printk(KERN_ERR "%s: Error attempting to initialize "
"the lower file for the dentry with name "
@@ -767,7 +767,7 @@ static int truncate_upper(struct dentry *dentry, struct iattr *ia,
lower_ia->ia_valid &= ~ATTR_SIZE;
return 0;
}
- rc = ecryptfs_get_lower_file(dentry, inode);
+ rc = ecryptfs_get_lower_file(dentry, inode, 0);
if (rc)
return rc;
crypt_stat = &ecryptfs_inode_to_private(dentry->d_inode)->crypt_stat;
@@ -935,7 +935,7 @@ static int ecryptfs_setattr(struct dentry *dentry, struct iattr *ia)
mount_crypt_stat = &ecryptfs_superblock_to_private(
dentry->d_sb)->mount_crypt_stat;
- rc = ecryptfs_get_lower_file(dentry, inode);
+ rc = ecryptfs_get_lower_file(dentry, inode, 0);
if (rc) {
mutex_unlock(&crypt_stat->cs_mutex);
goto out;
diff --git a/fs/ecryptfs/kthread.c b/fs/ecryptfs/kthread.c
index 809e67d..83e9914 100644
--- a/fs/ecryptfs/kthread.c
+++ b/fs/ecryptfs/kthread.c
@@ -122,6 +122,8 @@ void ecryptfs_destroy_kthread(void)
* @lower_file: Result of dentry_open by root on lower dentry
* @lower_dentry: Lower dentry for file to open
* @lower_mnt: Lower vfsmount for file to open
+ * @cred
+ * @upper_file_flags: Upper file flags for the open
*
* This function gets a r/w file opened againt the lower dentry.
*
@@ -130,7 +132,8 @@ void ecryptfs_destroy_kthread(void)
int ecryptfs_privileged_open(struct file **lower_file,
struct dentry *lower_dentry,
struct vfsmount *lower_mnt,
- const struct cred *cred)
+ const struct cred *cred,
+ unsigned int upper_file_flags)
{
struct ecryptfs_open_req req;
int flags = O_LARGEFILE;
@@ -145,6 +148,7 @@ int ecryptfs_privileged_open(struct file **lower_file,
* lower file is fput() when all eCryptfs files for the inode are
* released. */
flags |= IS_RDONLY(lower_dentry->d_inode) ? O_RDONLY : O_RDWR;
+ flags |= (upper_file_flags & FMODE_NONOTIFY);
(*lower_file) = dentry_open(&req.path, flags, cred);
if (!IS_ERR(*lower_file))
goto out;
diff --git a/fs/ecryptfs/main.c b/fs/ecryptfs/main.c
index 2768138..0ceebcc 100644
--- a/fs/ecryptfs/main.c
+++ b/fs/ecryptfs/main.c
@@ -117,7 +117,7 @@ void __ecryptfs_printk(const char *fmt, ...)
* Returns zero on success; non-zero otherwise
*/
static int ecryptfs_init_lower_file(struct dentry *dentry,
- struct file **lower_file)
+ struct file **lower_file, unsigned int upper_file_flags)
{
const struct cred *cred = current_cred();
struct dentry *lower_dentry = ecryptfs_dentry_to_lower(dentry);
@@ -125,7 +125,7 @@ static int ecryptfs_init_lower_file(struct dentry *dentry,
int rc;
rc = ecryptfs_privileged_open(lower_file, lower_dentry, lower_mnt,
- cred);
+ cred, upper_file_flags);
if (rc) {
printk(KERN_ERR "Error opening lower file "
"for lower_dentry [0x%p] and lower_mnt [0x%p]; "
@@ -135,7 +135,7 @@ static int ecryptfs_init_lower_file(struct dentry *dentry,
return rc;
}
-int ecryptfs_get_lower_file(struct dentry *dentry, struct inode *inode)
+int ecryptfs_get_lower_file(struct dentry *dentry, struct inode *inode, unsigned int upper_file_flags)
{
struct ecryptfs_inode_info *inode_info;
int count, rc = 0;
@@ -147,7 +147,7 @@ int ecryptfs_get_lower_file(struct dentry *dentry, struct inode *inode)
rc = -EINVAL;
else if (count == 1) {
rc = ecryptfs_init_lower_file(dentry,
- &inode_info->lower_file);
+ &inode_info->lower_file,upper_file_flags);
if (rc)
atomic_set(&inode_info->lower_file_count, 0);
}
--
1.7.9.5
^ permalink raw reply related [flat|nested] 7+ messages in thread
end of thread, other threads:[~2012-08-20 14:53 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
[not found] <5d3d0cdc-50db-4d60-8f9d-701acdbb86b0@ABN-EXCH1A.green.sophos>
[not found] ` <CANT6BaPhaAOv3hg9CgoDa9JTp9vN60zf8bdAQX+dFp6YLSUwnA@mail.gmail.com>
2012-08-14 9:00 ` ecryptfs and fanotify Douglas Leeder
[not found] ` <F85176E33E12BA45BD553324E7858B300CE0A5@abn-exch1b.green.sophos>
[not found] ` <F85176E33E12BA45BD553324E7858B300CE0A5-wyMY/JD5BThOaAOxIYwbkpbnZX9N9W2w@public.gmane.org>
2012-08-14 9:31 ` Florian Westphal
[not found] ` <20120814093124.GA12898@astaro.com>
[not found] ` <20120814093124.GA12898-OOs/4mkCeqbQT0dZR+AlfA@public.gmane.org>
2012-08-14 10:50 ` Douglas Leeder
[not found] ` <502A2D77.50805@sophos.com>
2012-08-14 16:01 ` Douglas Leeder
2012-08-14 20:27 ` Tyler C Hicks
2012-08-20 8:46 ` Douglas Leeder
2012-08-20 14:53 ` Douglas Leeder
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.