* [PATCH v3 1/1] vfs: iversion truncate bug fix @ 2011-12-22 10:54 Dmitry Kasatkin 2011-12-22 13:26 ` Mimi Zohar 0 siblings, 1 reply; 18+ messages in thread From: Dmitry Kasatkin @ 2011-12-22 10:54 UTC (permalink / raw) To: linux-fsdevel, linux-security-module Cc: akpm, viro, linux-kernel, zohar, Dmitry Kasatkin When a file is truncated with truncate()/ftruncate() and then closed, iversion is not updated. This patch uses ATTR_SIZE flag as an indication to increment iversion. Signed-off-by: Dmitry Kasatkin <dmitry.kasatkin@intel.com> --- fs/attr.c | 5 +++++ 1 files changed, 5 insertions(+), 0 deletions(-) diff --git a/fs/attr.c b/fs/attr.c index 7ee7ba4..b8f55c4 100644 --- a/fs/attr.c +++ b/fs/attr.c @@ -176,6 +176,11 @@ int notify_change(struct dentry * dentry, struct iattr * attr) return -EPERM; } + if ((ia_valid & ATTR_SIZE) && IS_I_VERSION(inode)) { + if (attr->ia_size != inode->i_size) + inode_inc_iversion(inode); + } + if ((ia_valid & ATTR_MODE)) { mode_t amode = attr->ia_mode; /* Flag setting protected by i_mutex */ -- 1.7.4.1 ^ permalink raw reply related [flat|nested] 18+ messages in thread
* Re: [PATCH v3 1/1] vfs: iversion truncate bug fix 2011-12-22 10:54 [PATCH v3 1/1] vfs: iversion truncate bug fix Dmitry Kasatkin @ 2011-12-22 13:26 ` Mimi Zohar 2012-01-04 23:28 ` Andrew Morton 0 siblings, 1 reply; 18+ messages in thread From: Mimi Zohar @ 2011-12-22 13:26 UTC (permalink / raw) To: Dmitry Kasatkin Cc: linux-fsdevel, linux-security-module, akpm, viro, linux-kernel On Thu, 2011-12-22 at 12:54 +0200, Dmitry Kasatkin wrote: > When a file is truncated with truncate()/ftruncate() and then closed, > iversion is not updated. This patch uses ATTR_SIZE flag as an indication > to increment iversion. > > Signed-off-by: Dmitry Kasatkin <dmitry.kasatkin@intel.com> Acked-by: Mimi Zohar <zohar@us.ibm.com> (Stable should be cc'ed on this patch.) thanks, Mimi > --- > fs/attr.c | 5 +++++ > 1 files changed, 5 insertions(+), 0 deletions(-) > > diff --git a/fs/attr.c b/fs/attr.c > index 7ee7ba4..b8f55c4 100644 > --- a/fs/attr.c > +++ b/fs/attr.c > @@ -176,6 +176,11 @@ int notify_change(struct dentry * dentry, struct iattr * attr) > return -EPERM; > } > > + if ((ia_valid & ATTR_SIZE) && IS_I_VERSION(inode)) { > + if (attr->ia_size != inode->i_size) > + inode_inc_iversion(inode); > + } > + > if ((ia_valid & ATTR_MODE)) { > mode_t amode = attr->ia_mode; > /* Flag setting protected by i_mutex */ ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH v3 1/1] vfs: iversion truncate bug fix 2011-12-22 13:26 ` Mimi Zohar @ 2012-01-04 23:28 ` Andrew Morton 2012-01-05 0:33 ` Mimi Zohar 0 siblings, 1 reply; 18+ messages in thread From: Andrew Morton @ 2012-01-04 23:28 UTC (permalink / raw) To: Mimi Zohar Cc: Dmitry Kasatkin, linux-fsdevel, linux-security-module, viro, linux-kernel On Thu, 22 Dec 2011 08:26:30 -0500 Mimi Zohar <zohar@linux.vnet.ibm.com> wrote: > On Thu, 2011-12-22 at 12:54 +0200, Dmitry Kasatkin wrote: > > When a file is truncated with truncate()/ftruncate() and then closed, > > iversion is not updated. This patch uses ATTR_SIZE flag as an indication > > to increment iversion. > > > > Signed-off-by: Dmitry Kasatkin <dmitry.kasatkin@intel.com> > > Acked-by: Mimi Zohar <zohar@us.ibm.com> > (Stable should be cc'ed on this patch.) why? ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH v3 1/1] vfs: iversion truncate bug fix 2012-01-04 23:28 ` Andrew Morton @ 2012-01-05 0:33 ` Mimi Zohar 2012-01-05 0:46 ` Andrew Morton 0 siblings, 1 reply; 18+ messages in thread From: Mimi Zohar @ 2012-01-05 0:33 UTC (permalink / raw) To: Andrew Morton Cc: Dmitry Kasatkin, linux-fsdevel, linux-security-module, viro, linux-kernel On Wed, 2012-01-04 at 15:28 -0800, Andrew Morton wrote: > On Thu, 22 Dec 2011 08:26:30 -0500 > Mimi Zohar <zohar@linux.vnet.ibm.com> wrote: > > > On Thu, 2011-12-22 at 12:54 +0200, Dmitry Kasatkin wrote: > > > When a file is truncated with truncate()/ftruncate() and then closed, > > > iversion is not updated. This patch uses ATTR_SIZE flag as an indication > > > to increment iversion. > > > > > > Signed-off-by: Dmitry Kasatkin <dmitry.kasatkin@intel.com> > > > > Acked-by: Mimi Zohar <zohar@us.ibm.com> > > (Stable should be cc'ed on this patch.) > > why? Why backported? The IMA measurement list could be incomplete. Mimi ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH v3 1/1] vfs: iversion truncate bug fix 2012-01-05 0:33 ` Mimi Zohar @ 2012-01-05 0:46 ` Andrew Morton 2012-01-05 2:06 ` Greg KH 0 siblings, 1 reply; 18+ messages in thread From: Andrew Morton @ 2012-01-05 0:46 UTC (permalink / raw) To: Mimi Zohar Cc: Dmitry Kasatkin, linux-fsdevel, linux-security-module, viro, linux-kernel On Wed, 04 Jan 2012 19:33:49 -0500 Mimi Zohar <zohar@linux.vnet.ibm.com> wrote: > On Wed, 2012-01-04 at 15:28 -0800, Andrew Morton wrote: > > On Thu, 22 Dec 2011 08:26:30 -0500 > > Mimi Zohar <zohar@linux.vnet.ibm.com> wrote: > > > > > On Thu, 2011-12-22 at 12:54 +0200, Dmitry Kasatkin wrote: > > > > When a file is truncated with truncate()/ftruncate() and then closed, > > > > iversion is not updated. This patch uses ATTR_SIZE flag as an indication > > > > to increment iversion. > > > > > > > > Signed-off-by: Dmitry Kasatkin <dmitry.kasatkin@intel.com> > > > > > > Acked-by: Mimi Zohar <zohar@us.ibm.com> > > > (Stable should be cc'ed on this patch.) > > > > why? > > Why backported? Yes. If you want to submit the patch to the -stable maintainer then you should explain to him why the fix is important enough to warrant doing that. That involves explaining the user-visible effects of the bug. > The IMA measurement list could be incomplete. In more detail than this. Maybe he knows what the above sentence means, but I sure don't. ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH v3 1/1] vfs: iversion truncate bug fix 2012-01-05 0:46 ` Andrew Morton @ 2012-01-05 2:06 ` Greg KH 2012-01-05 4:17 ` Mimi Zohar 0 siblings, 1 reply; 18+ messages in thread From: Greg KH @ 2012-01-05 2:06 UTC (permalink / raw) To: Andrew Morton Cc: Mimi Zohar, Dmitry Kasatkin, linux-fsdevel, linux-security-module, viro, linux-kernel On Wed, Jan 04, 2012 at 04:46:38PM -0800, Andrew Morton wrote: > On Wed, 04 Jan 2012 19:33:49 -0500 > Mimi Zohar <zohar@linux.vnet.ibm.com> wrote: > > > On Wed, 2012-01-04 at 15:28 -0800, Andrew Morton wrote: > > > On Thu, 22 Dec 2011 08:26:30 -0500 > > > Mimi Zohar <zohar@linux.vnet.ibm.com> wrote: > > > > > > > On Thu, 2011-12-22 at 12:54 +0200, Dmitry Kasatkin wrote: > > > > > When a file is truncated with truncate()/ftruncate() and then closed, > > > > > iversion is not updated. This patch uses ATTR_SIZE flag as an indication > > > > > to increment iversion. > > > > > > > > > > Signed-off-by: Dmitry Kasatkin <dmitry.kasatkin@intel.com> > > > > > > > > Acked-by: Mimi Zohar <zohar@us.ibm.com> > > > > (Stable should be cc'ed on this patch.) > > > > > > why? > > > > Why backported? > > Yes. If you want to submit the patch to the -stable maintainer then > you should explain to him why the fix is important enough to warrant > doing that. That involves explaining the user-visible effects of > the bug. > > > The IMA measurement list could be incomplete. > > In more detail than this. Maybe he knows what the above sentence > means, but I sure don't. Nope, I don't either :) ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH v3 1/1] vfs: iversion truncate bug fix 2012-01-05 2:06 ` Greg KH @ 2012-01-05 4:17 ` Mimi Zohar 2012-01-05 4:53 ` Dave Chinner 2012-01-05 16:54 ` Greg KH 0 siblings, 2 replies; 18+ messages in thread From: Mimi Zohar @ 2012-01-05 4:17 UTC (permalink / raw) To: Greg KH Cc: Andrew Morton, Dmitry Kasatkin, linux-fsdevel, linux-security-module, viro, linux-kernel On Wed, 2012-01-04 at 18:06 -0800, Greg KH wrote: > On Wed, Jan 04, 2012 at 04:46:38PM -0800, Andrew Morton wrote: > > On Wed, 04 Jan 2012 19:33:49 -0500 > > Mimi Zohar <zohar@linux.vnet.ibm.com> wrote: > > > > > On Wed, 2012-01-04 at 15:28 -0800, Andrew Morton wrote: > > > > On Thu, 22 Dec 2011 08:26:30 -0500 > > > > Mimi Zohar <zohar@linux.vnet.ibm.com> wrote: > > > > > > > > > On Thu, 2011-12-22 at 12:54 +0200, Dmitry Kasatkin wrote: > > > > > > When a file is truncated with truncate()/ftruncate() and then closed, > > > > > > iversion is not updated. This patch uses ATTR_SIZE flag as an indication > > > > > > to increment iversion. > > > > > > > > > > > > Signed-off-by: Dmitry Kasatkin <dmitry.kasatkin@intel.com> > > > > > > > > > > Acked-by: Mimi Zohar <zohar@us.ibm.com> > > > > > (Stable should be cc'ed on this patch.) > > > > > > > > why? > > > > > > Why backported? > > > > Yes. If you want to submit the patch to the -stable maintainer then > > you should explain to him why the fix is important enough to warrant > > doing that. That involves explaining the user-visible effects of > > the bug. > > > > > The IMA measurement list could be incomplete. > > > > In more detail than this. Maybe he knows what the above sentence > > means, but I sure don't. > > Nope, I don't either :) On fput(), i_version is used to detect and flag files that have changed and need to be re-measured in the IMA measurement policy. When a file is truncated with truncate()/ftruncate() and then closed, i_version is not updated. As a result, although the file has changed, it will not be re-measured and added to the IMA measurement list on subsequent access. Mimi ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH v3 1/1] vfs: iversion truncate bug fix 2012-01-05 4:17 ` Mimi Zohar @ 2012-01-05 4:53 ` Dave Chinner 2012-01-05 18:14 ` Christoph Hellwig 2012-01-05 16:54 ` Greg KH 1 sibling, 1 reply; 18+ messages in thread From: Dave Chinner @ 2012-01-05 4:53 UTC (permalink / raw) To: Mimi Zohar Cc: Greg KH, Andrew Morton, Dmitry Kasatkin, linux-fsdevel, linux-security-module, viro, linux-kernel On Wed, Jan 04, 2012 at 11:17:12PM -0500, Mimi Zohar wrote: > On Wed, 2012-01-04 at 18:06 -0800, Greg KH wrote: > > On Wed, Jan 04, 2012 at 04:46:38PM -0800, Andrew Morton wrote: > > > On Wed, 04 Jan 2012 19:33:49 -0500 > > > Mimi Zohar <zohar@linux.vnet.ibm.com> wrote: > > > > > > > On Wed, 2012-01-04 at 15:28 -0800, Andrew Morton wrote: > > > > > On Thu, 22 Dec 2011 08:26:30 -0500 > > > > > Mimi Zohar <zohar@linux.vnet.ibm.com> wrote: > > > > > > > > > > > On Thu, 2011-12-22 at 12:54 +0200, Dmitry Kasatkin wrote: > > > > > > > When a file is truncated with truncate()/ftruncate() and then closed, > > > > > > > iversion is not updated. This patch uses ATTR_SIZE flag as an indication > > > > > > > to increment iversion. > > > > > > > > > > > > > > Signed-off-by: Dmitry Kasatkin <dmitry.kasatkin@intel.com> > > > > > > > > > > > > Acked-by: Mimi Zohar <zohar@us.ibm.com> > > > > > > (Stable should be cc'ed on this patch.) > > > > > > > > > > why? > > > > > > > > Why backported? > > > > > > Yes. If you want to submit the patch to the -stable maintainer then > > > you should explain to him why the fix is important enough to warrant > > > doing that. That involves explaining the user-visible effects of > > > the bug. > > > > > > > The IMA measurement list could be incomplete. > > > > > > In more detail than this. Maybe he knows what the above sentence > > > means, but I sure don't. > > > > Nope, I don't either :) > > On fput(), i_version is used to detect and flag files that have changed > and need to be re-measured in the IMA measurement policy. When a file > is truncated with truncate()/ftruncate() and then closed, i_version is > not updated. As a result, although the file has changed, it will not be > re-measured and added to the IMA measurement list on subsequent access. That's seems like a rather unreliable way of detecting that a file has changed to me. I mean, only ext4 uses inode_inc_version() when it internally dirties an inode, and only ext4 sets the MS_I_VERSION so that inode_inc_version is only called for ext4 inodes when timestamps change. Other filesystems randomly open code i_version inrements, and many don't touch it at all when inodes are changed, so it this really doesn't seem like anything anyone can rely on for detecting changes except maybe for ext4 users. Hence just adding an increment to the truncate case doesn't seem to be sufficient to me. e.g. what about the equivalent case of having a hole punched in the file via fallocate? The file has definitely changed, but i_version won't change.... Perhaps bumping i_version in __mark_inode_dirty() might be the best way to capture all changes (other than timestamp updates) to any inode regardless of the filesystem type? Cheers, Dave. -- Dave Chinner david@fromorbit.com ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH v3 1/1] vfs: iversion truncate bug fix 2012-01-05 4:53 ` Dave Chinner @ 2012-01-05 18:14 ` Christoph Hellwig 2012-01-05 19:49 ` Mimi Zohar ` (2 more replies) 0 siblings, 3 replies; 18+ messages in thread From: Christoph Hellwig @ 2012-01-05 18:14 UTC (permalink / raw) To: Dave Chinner Cc: Mimi Zohar, Greg KH, Andrew Morton, Dmitry Kasatkin, linux-fsdevel, linux-security-module, viro, linux-kernel On Thu, Jan 05, 2012 at 03:53:54PM +1100, Dave Chinner wrote: > That's seems like a rather unreliable way of detecting that a file > has changed to me. I mean, only ext4 uses inode_inc_version() when > it internally dirties an inode, and only ext4 sets the MS_I_VERSION > so that inode_inc_version is only called for ext4 inodes when > timestamps change. And even ext4 only does it when using the non-default "i_version" mount option. > Hence just adding an increment to the truncate case doesn't seem to > be sufficient to me. e.g. what about the equivalent case of having a > hole punched in the file via fallocate? The file has definitely > changed, but i_version won't change.... > > Perhaps bumping i_version in __mark_inode_dirty() might be the best > way to capture all changes (other than timestamp updates) to any > inode regardless of the filesystem type? It has the same problem as the timestamp updates doing that right now - the fs can't do locking around it, and it can't return errors. That's something affecting at least btrfs, xfs and IIRC ubifs, and probably the cluster filesystems as well. The right answer is to replace the timestmap updates which are the only places doing that with a method as Josef had planned to do, and then we can include the i_version updates in there. That assumes we figure out a coherent way to do it - except for the conditional abuse in file_updates_times it's currently entirely under fs control. So the best way to fix it would be to: - move the fs-private use into those filesystems actually using it. Note that a lot less actually check for it rather than just updating it based on some cargo cult, and most only do so for directories. - figure a why what exact change count semantics NFS (and IMA) want and find a way to implement them so that the fs can tell the callers that they don't exist. Btw, does IMA care about these beeing persistent? ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH v3 1/1] vfs: iversion truncate bug fix 2012-01-05 18:14 ` Christoph Hellwig @ 2012-01-05 19:49 ` Mimi Zohar 2012-01-05 21:36 ` J. Bruce Fields 2012-01-05 22:27 ` Mimi Zohar 2 siblings, 0 replies; 18+ messages in thread From: Mimi Zohar @ 2012-01-05 19:49 UTC (permalink / raw) To: Christoph Hellwig Cc: Dave Chinner, Greg KH, Andrew Morton, Dmitry Kasatkin, linux-fsdevel, linux-security-module, viro, linux-kernel On Thu, 2012-01-05 at 13:14 -0500, Christoph Hellwig wrote: > On Thu, Jan 05, 2012 at 03:53:54PM +1100, Dave Chinner wrote: > > That's seems like a rather unreliable way of detecting that a file > > has changed to me. I mean, only ext4 uses inode_inc_version() when > > it internally dirties an inode, and only ext4 sets the MS_I_VERSION > > so that inode_inc_version is only called for ext4 inodes when > > timestamps change. > > And even ext4 only does it when using the non-default "i_version" > mount option. > > > Hence just adding an increment to the truncate case doesn't seem to > > be sufficient to me. e.g. what about the equivalent case of having a > > hole punched in the file via fallocate? The file has definitely > > changed, but i_version won't change.... > > > > Perhaps bumping i_version in __mark_inode_dirty() might be the best > > way to capture all changes (other than timestamp updates) to any > > inode regardless of the filesystem type? > > It has the same problem as the timestamp updates doing that right now - > the fs can't do locking around it, and it can't return errors. That's > something affecting at least btrfs, xfs and IIRC ubifs, and probably > the cluster filesystems as well. The right answer is to replace the > timestmap updates which are the only places doing that with a method > as Josef had planned to do, and then we can include the i_version > updates in there. > > That assumes we figure out a coherent way to do it - except for the > conditional abuse in file_updates_times it's currently entirely under > fs control. So the best way to fix it would be to: > > - move the fs-private use into those filesystems actually using it. > Note that a lot less actually check for it rather than just updating > it based on some cargo cult, and most only do so for directories. > - figure a why what exact change count semantics NFS (and IMA) want > and find a way to implement them so that the fs can tell the callers > that they don't exist. > > Btw, does IMA care about these beeing persistent? By 'persistent' I assume you mean across boots. IMA (and IMA-appraisal) measure and appraise files the first time they're accessed/executed. So no, it does not need to be persistent. IMA/IMA-appraisal just need some way to detect file change in order to know whether the file needs to be re-measured/appraised on subsequent access. Mimi ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH v3 1/1] vfs: iversion truncate bug fix 2012-01-05 18:14 ` Christoph Hellwig 2012-01-05 19:49 ` Mimi Zohar @ 2012-01-05 21:36 ` J. Bruce Fields 2012-01-05 22:27 ` Mimi Zohar 2 siblings, 0 replies; 18+ messages in thread From: J. Bruce Fields @ 2012-01-05 21:36 UTC (permalink / raw) To: Christoph Hellwig Cc: Dave Chinner, Mimi Zohar, Greg KH, Andrew Morton, Dmitry Kasatkin, linux-fsdevel, linux-security-module, viro, linux-kernel On Thu, Jan 05, 2012 at 01:14:01PM -0500, Christoph Hellwig wrote: > On Thu, Jan 05, 2012 at 03:53:54PM +1100, Dave Chinner wrote: > > That's seems like a rather unreliable way of detecting that a file > > has changed to me. I mean, only ext4 uses inode_inc_version() when > > it internally dirties an inode, and only ext4 sets the MS_I_VERSION > > so that inode_inc_version is only called for ext4 inodes when > > timestamps change. > > And even ext4 only does it when using the non-default "i_version" > mount option. > > > Hence just adding an increment to the truncate case doesn't seem to > > be sufficient to me. e.g. what about the equivalent case of having a > > hole punched in the file via fallocate? The file has definitely > > changed, but i_version won't change.... > > > > Perhaps bumping i_version in __mark_inode_dirty() might be the best > > way to capture all changes (other than timestamp updates) to any > > inode regardless of the filesystem type? > > It has the same problem as the timestamp updates doing that right now - > the fs can't do locking around it, and it can't return errors. That's > something affecting at least btrfs, xfs and IIRC ubifs, and probably > the cluster filesystems as well. The right answer is to replace the > timestmap updates which are the only places doing that with a method > as Josef had planned to do, and then we can include the i_version > updates in there. > > That assumes we figure out a coherent way to do it - except for the > conditional abuse in file_updates_times it's currently entirely under > fs control. So the best way to fix it would be to: > > - move the fs-private use into those filesystems actually using it. > Note that a lot less actually check for it rather than just updating > it based on some cargo cult, and most only do so for directories. > - figure a why what exact change count semantics NFS (and IMA) want For NFS: The NFSv4 protocol has a "change attribute" that a client should be able to use determine whether its cache is up to date. It therefore doesn't have to be a "count" necessarily, as long as it changes every time. (And is extremely unlikely to repeat.) It should be on by default for any exportable filesystem capable of supporting it. It has to change on both data and metadata changes. (Like ctime.) It has to work for both files and directories. It has to persist on disk--a client should be able to revalidate its cache by comparing change attributes fetched before and after a server reboot. (But: I don't know if there's any easy way to avoid the corner case where unsync'd updates cause i_version to go backwards after reboot, and then to repeat a previous i_version value after subsequent updates.) --b. > and find a way to implement them so that the fs can tell the callers > that they don't exist. > > Btw, does IMA care about these beeing persistent? > -- > To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH v3 1/1] vfs: iversion truncate bug fix 2012-01-05 18:14 ` Christoph Hellwig 2012-01-05 19:49 ` Mimi Zohar 2012-01-05 21:36 ` J. Bruce Fields @ 2012-01-05 22:27 ` Mimi Zohar 2 siblings, 0 replies; 18+ messages in thread From: Mimi Zohar @ 2012-01-05 22:27 UTC (permalink / raw) To: Christoph Hellwig Cc: Dave Chinner, Greg KH, Andrew Morton, Dmitry Kasatkin, linux-fsdevel, linux-security-module, viro, linux-kernel On Thu, 2012-01-05 at 14:49 -0500, Mimi Zohar wrote: > On Thu, 2012-01-05 at 13:14 -0500, Christoph Hellwig wrote: > > On Thu, Jan 05, 2012 at 03:53:54PM +1100, Dave Chinner wrote: > > > That's seems like a rather unreliable way of detecting that a file > > > has changed to me. I mean, only ext4 uses inode_inc_version() when > > > it internally dirties an inode, and only ext4 sets the MS_I_VERSION > > > so that inode_inc_version is only called for ext4 inodes when > > > timestamps change. > > > > And even ext4 only does it when using the non-default "i_version" > > mount option. > > > > > Hence just adding an increment to the truncate case doesn't seem to > > > be sufficient to me. e.g. what about the equivalent case of having a > > > hole punched in the file via fallocate? The file has definitely > > > changed, but i_version won't change.... > > > > > > Perhaps bumping i_version in __mark_inode_dirty() might be the best > > > way to capture all changes (other than timestamp updates) to any > > > inode regardless of the filesystem type? > > > > It has the same problem as the timestamp updates doing that right now - > > the fs can't do locking around it, and it can't return errors. That's > > something affecting at least btrfs, xfs and IIRC ubifs, and probably > > the cluster filesystems as well. The right answer is to replace the > > timestmap updates which are the only places doing that with a method > > as Josef had planned to do, and then we can include the i_version > > updates in there. > > > > That assumes we figure out a coherent way to do it - except for the > > conditional abuse in file_updates_times it's currently entirely under > > fs control. So the best way to fix it would be to: > > > > - move the fs-private use into those filesystems actually using it. > > Note that a lot less actually check for it rather than just updating > > it based on some cargo cult, and most only do so for directories. > > - figure a why what exact change count semantics NFS (and IMA) want > > and find a way to implement them so that the fs can tell the callers > > that they don't exist. > > > > Btw, does IMA care about these beeing persistent? > > By 'persistent' I assume you mean across boots. IMA (and IMA-appraisal) > measure and appraise files the first time they're accessed/executed. So > no, it does not need to be persistent. IMA/IMA-appraisal just need some > way to detect file change in order to know whether the file needs to be > re-measured/appraised on subsequent access. One other IMA requirement would be the ability to detect if a file mmapped executable, after the point that it has been locked from modification, has changed since the last time measured/appraised. The concern here, in particular, is the ability to detect file change while holding the mmap_sem. thanks, Mimi ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH v3 1/1] vfs: iversion truncate bug fix 2012-01-05 4:17 ` Mimi Zohar 2012-01-05 4:53 ` Dave Chinner @ 2012-01-05 16:54 ` Greg KH 2012-01-05 17:19 ` Mimi Zohar 1 sibling, 1 reply; 18+ messages in thread From: Greg KH @ 2012-01-05 16:54 UTC (permalink / raw) To: Mimi Zohar Cc: Andrew Morton, Dmitry Kasatkin, linux-fsdevel, linux-security-module, viro, linux-kernel On Wed, Jan 04, 2012 at 11:17:12PM -0500, Mimi Zohar wrote: > On Wed, 2012-01-04 at 18:06 -0800, Greg KH wrote: > > On Wed, Jan 04, 2012 at 04:46:38PM -0800, Andrew Morton wrote: > > > On Wed, 04 Jan 2012 19:33:49 -0500 > > > Mimi Zohar <zohar@linux.vnet.ibm.com> wrote: > > > > > > > On Wed, 2012-01-04 at 15:28 -0800, Andrew Morton wrote: > > > > > On Thu, 22 Dec 2011 08:26:30 -0500 > > > > > Mimi Zohar <zohar@linux.vnet.ibm.com> wrote: > > > > > > > > > > > On Thu, 2011-12-22 at 12:54 +0200, Dmitry Kasatkin wrote: > > > > > > > When a file is truncated with truncate()/ftruncate() and then closed, > > > > > > > iversion is not updated. This patch uses ATTR_SIZE flag as an indication > > > > > > > to increment iversion. > > > > > > > > > > > > > > Signed-off-by: Dmitry Kasatkin <dmitry.kasatkin@intel.com> > > > > > > > > > > > > Acked-by: Mimi Zohar <zohar@us.ibm.com> > > > > > > (Stable should be cc'ed on this patch.) > > > > > > > > > > why? > > > > > > > > Why backported? > > > > > > Yes. If you want to submit the patch to the -stable maintainer then > > > you should explain to him why the fix is important enough to warrant > > > doing that. That involves explaining the user-visible effects of > > > the bug. > > > > > > > The IMA measurement list could be incomplete. > > > > > > In more detail than this. Maybe he knows what the above sentence > > > means, but I sure don't. > > > > Nope, I don't either :) > > On fput(), i_version is used to detect and flag files that have changed > and need to be re-measured in the IMA measurement policy. When a file > is truncated with truncate()/ftruncate() and then closed, i_version is > not updated. As a result, although the file has changed, it will not be > re-measured and added to the IMA measurement list on subsequent access. And what am I supposed to do with this? Please, go read Documentation/stable_kernel_rules.txt for how to properly submit patches to the stable kernel tree. The information here needs to be in the patch changelog itself, not in some random email thread that will get lost instantly into my email-archive-from-hell after I am done reading this. greg k-h ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH v3 1/1] vfs: iversion truncate bug fix 2012-01-05 16:54 ` Greg KH @ 2012-01-05 17:19 ` Mimi Zohar 2012-01-05 18:39 ` James Bottomley 0 siblings, 1 reply; 18+ messages in thread From: Mimi Zohar @ 2012-01-05 17:19 UTC (permalink / raw) To: Greg KH Cc: Andrew Morton, Dmitry Kasatkin, linux-fsdevel, linux-security-module, viro, linux-kernel On Thu, 2012-01-05 at 08:54 -0800, Greg KH wrote: > On Wed, Jan 04, 2012 at 11:17:12PM -0500, Mimi Zohar wrote: > > On Wed, 2012-01-04 at 18:06 -0800, Greg KH wrote: > > > On Wed, Jan 04, 2012 at 04:46:38PM -0800, Andrew Morton wrote: > > > > On Wed, 04 Jan 2012 19:33:49 -0500 > > > > Mimi Zohar <zohar@linux.vnet.ibm.com> wrote: > > > > > > > > > On Wed, 2012-01-04 at 15:28 -0800, Andrew Morton wrote: > > > > > > On Thu, 22 Dec 2011 08:26:30 -0500 > > > > > > Mimi Zohar <zohar@linux.vnet.ibm.com> wrote: > > > > > > > > > > > > > On Thu, 2011-12-22 at 12:54 +0200, Dmitry Kasatkin wrote: > > > > > > > > When a file is truncated with truncate()/ftruncate() and then closed, > > > > > > > > iversion is not updated. This patch uses ATTR_SIZE flag as an indication > > > > > > > > to increment iversion. > > > > > > > > > > > > > > > > Signed-off-by: Dmitry Kasatkin <dmitry.kasatkin@intel.com> > > > > > > > > > > > > > > Acked-by: Mimi Zohar <zohar@us.ibm.com> > > > > > > > (Stable should be cc'ed on this patch.) > > > > > > > > > > > > why? > > > > > > > > > > Why backported? > > > > > > > > Yes. If you want to submit the patch to the -stable maintainer then > > > > you should explain to him why the fix is important enough to warrant > > > > doing that. That involves explaining the user-visible effects of > > > > the bug. > > > > > > > > > The IMA measurement list could be incomplete. > > > > > > > > In more detail than this. Maybe he knows what the above sentence > > > > means, but I sure don't. > > > > > > Nope, I don't either :) > > > > On fput(), i_version is used to detect and flag files that have changed > > and need to be re-measured in the IMA measurement policy. When a file > > is truncated with truncate()/ftruncate() and then closed, i_version is > > not updated. As a result, although the file has changed, it will not be > > re-measured and added to the IMA measurement list on subsequent access. > > And what am I supposed to do with this? > > Please, go read Documentation/stable_kernel_rules.txt for how to > properly submit patches to the stable kernel tree. The information here > needs to be in the patch changelog itself, not in some random email > thread that will get lost instantly into my email-archive-from-hell > after I am done reading this. > > greg k-h Yes, I've read Documentation/stable_kernel_rules.txt and think this patch meets the criteria for being backported. As far as I'm aware, this patch hasn't been upstreamed yet and is waiting for someone, besides myself, to Ack it. Once Acked, either Dmitry or I can send a pull request with an updated patch description. Should this patch go in via the security tree? thanks, Mimi ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH v3 1/1] vfs: iversion truncate bug fix 2012-01-05 17:19 ` Mimi Zohar @ 2012-01-05 18:39 ` James Bottomley 2012-01-05 22:30 ` Andrew Morton 0 siblings, 1 reply; 18+ messages in thread From: James Bottomley @ 2012-01-05 18:39 UTC (permalink / raw) To: Mimi Zohar Cc: Greg KH, Andrew Morton, Dmitry Kasatkin, linux-fsdevel, linux-security-module, viro, linux-kernel On Thu, 2012-01-05 at 12:19 -0500, Mimi Zohar wrote: > On Thu, 2012-01-05 at 08:54 -0800, Greg KH wrote: > > On Wed, Jan 04, 2012 at 11:17:12PM -0500, Mimi Zohar wrote: > > > On Wed, 2012-01-04 at 18:06 -0800, Greg KH wrote: > > > > On Wed, Jan 04, 2012 at 04:46:38PM -0800, Andrew Morton wrote: > > > > > On Wed, 04 Jan 2012 19:33:49 -0500 > > > > > Mimi Zohar <zohar@linux.vnet.ibm.com> wrote: > > > > > > > > > > > On Wed, 2012-01-04 at 15:28 -0800, Andrew Morton wrote: > > > > > > > On Thu, 22 Dec 2011 08:26:30 -0500 > > > > > > > Mimi Zohar <zohar@linux.vnet.ibm.com> wrote: > > > > > > > > > > > > > > > On Thu, 2011-12-22 at 12:54 +0200, Dmitry Kasatkin wrote: > > > > > > > > > When a file is truncated with truncate()/ftruncate() and then closed, > > > > > > > > > iversion is not updated. This patch uses ATTR_SIZE flag as an indication > > > > > > > > > to increment iversion. > > > > > > > > > > > > > > > > > > Signed-off-by: Dmitry Kasatkin <dmitry.kasatkin@intel.com> > > > > > > > > > > > > > > > > Acked-by: Mimi Zohar <zohar@us.ibm.com> > > > > > > > > (Stable should be cc'ed on this patch.) > > > > > > > > > > > > > > why? > > > > > > > > > > > > Why backported? > > > > > > > > > > Yes. If you want to submit the patch to the -stable maintainer then > > > > > you should explain to him why the fix is important enough to warrant > > > > > doing that. That involves explaining the user-visible effects of > > > > > the bug. > > > > > > > > > > > The IMA measurement list could be incomplete. > > > > > > > > > > In more detail than this. Maybe he knows what the above sentence > > > > > means, but I sure don't. > > > > > > > > Nope, I don't either :) > > > > > > On fput(), i_version is used to detect and flag files that have changed > > > and need to be re-measured in the IMA measurement policy. When a file > > > is truncated with truncate()/ftruncate() and then closed, i_version is > > > not updated. As a result, although the file has changed, it will not be > > > re-measured and added to the IMA measurement list on subsequent access. > > > > And what am I supposed to do with this? > > > > Please, go read Documentation/stable_kernel_rules.txt for how to > > properly submit patches to the stable kernel tree. The information here > > needs to be in the patch changelog itself, not in some random email > > thread that will get lost instantly into my email-archive-from-hell > > after I am done reading this. > > > > greg k-h > > Yes, I've read Documentation/stable_kernel_rules.txt and think this > patch meets the criteria for being backported. > > As far as I'm aware, this patch hasn't been upstreamed yet and is > waiting for someone, besides myself, to Ack it. Once Acked, either > Dmitry or I can send a pull request with an updated patch description. > Should this patch go in via the security tree? If it hasn't been upstreamed yet, just make sure you put cc: stable@kernel.org In the signoffs of the patch you're sending upstream and the backport will occur automatically when the patch is finally upstreamed. James ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH v3 1/1] vfs: iversion truncate bug fix 2012-01-05 18:39 ` James Bottomley @ 2012-01-05 22:30 ` Andrew Morton 2012-01-05 23:09 ` Mimi Zohar 2012-01-13 10:13 ` Kasatkin, Dmitry 0 siblings, 2 replies; 18+ messages in thread From: Andrew Morton @ 2012-01-05 22:30 UTC (permalink / raw) To: James Bottomley Cc: Mimi Zohar, Greg KH, Dmitry Kasatkin, linux-fsdevel, linux-security-module, viro, linux-kernel On Thu, 05 Jan 2012 10:39:41 -0800 James Bottomley <James.Bottomley@HansenPartnership.com> wrote: > > > Please, go read Documentation/stable_kernel_rules.txt for how to > > > properly submit patches to the stable kernel tree. The information here > > > needs to be in the patch changelog itself, not in some random email > > > thread that will get lost instantly into my email-archive-from-hell > > > after I am done reading this. > > > > > > greg k-h > > > > Yes, I've read Documentation/stable_kernel_rules.txt and think this > > patch meets the criteria for being backported. > > > > As far as I'm aware, this patch hasn't been upstreamed yet and is > > waiting for someone, besides myself, to Ack it. Once Acked, either > > Dmitry or I can send a pull request with an updated patch description. > > Should this patch go in via the security tree? > > If it hasn't been upstreamed yet, just make sure you put > > cc: stable@kernel.org > > In the signoffs of the patch you're sending upstream and the backport > will occur automatically when the patch is finally upstreamed. Mimi didn't write or send the patch. This happily git-free maintainer simply goes in and edits the changelog. I do this very very frequently. Here's what I currently have. I plan to send this to Rip Van Viro. From: Dmitry Kasatkin <dmitry.kasatkin@intel.com> Subject: vfs: increment iversion when a file is truncated When a file is truncated with truncate()/ftruncate() and then closed, iversion is not updated. This patch uses ATTR_SIZE flag as an indication to increment iversion. Mimi said: On fput(), i_version is used to detect and flag files that have changed and need to be re-measured in the IMA measurement policy. When a file is truncated with truncate()/ftruncate() and then closed, i_version is not updated. As a result, although the file has changed, it will not be re-measured and added to the IMA measurement list on subsequent access. Signed-off-by: Dmitry Kasatkin <dmitry.kasatkin@intel.com> Acked-by: Mimi Zohar <zohar@us.ibm.com> Cc: Al Viro <viro@zeniv.linux.org.uk> Cc: <stable@vger.kernel.org> Signed-off-by: Andrew Morton <akpm@linux-foundation.org> --- fs/attr.c | 5 +++++ 1 file changed, 5 insertions(+) diff -puN fs/attr.c~vfs-increment-iversion-when-a-file-is-truncated fs/attr.c --- a/fs/attr.c~vfs-increment-iversion-when-a-file-is-truncated +++ a/fs/attr.c @@ -176,6 +176,11 @@ int notify_change(struct dentry * dentry return -EPERM; } + if ((ia_valid & ATTR_SIZE) && IS_I_VERSION(inode)) { + if (attr->ia_size != inode->i_size) + inode_inc_iversion(inode); + } + if ((ia_valid & ATTR_MODE)) { umode_t amode = attr->ia_mode; /* Flag setting protected by i_mutex */ _ ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH v3 1/1] vfs: iversion truncate bug fix 2012-01-05 22:30 ` Andrew Morton @ 2012-01-05 23:09 ` Mimi Zohar 2012-01-13 10:13 ` Kasatkin, Dmitry 1 sibling, 0 replies; 18+ messages in thread From: Mimi Zohar @ 2012-01-05 23:09 UTC (permalink / raw) To: Andrew Morton Cc: James Bottomley, Greg KH, Dmitry Kasatkin, linux-fsdevel, linux-security-module, viro, linux-kernel On Thu, 2012-01-05 at 14:30 -0800, Andrew Morton wrote: > On Thu, 05 Jan 2012 10:39:41 -0800 > James Bottomley <James.Bottomley@HansenPartnership.com> wrote: > > > > > Please, go read Documentation/stable_kernel_rules.txt for how to > > > > properly submit patches to the stable kernel tree. The information here > > > > needs to be in the patch changelog itself, not in some random email > > > > thread that will get lost instantly into my email-archive-from-hell > > > > after I am done reading this. > > > > > > > > greg k-h > > > > > > Yes, I've read Documentation/stable_kernel_rules.txt and think this > > > patch meets the criteria for being backported. > > > > > > As far as I'm aware, this patch hasn't been upstreamed yet and is > > > waiting for someone, besides myself, to Ack it. Once Acked, either > > > Dmitry or I can send a pull request with an updated patch description. > > > Should this patch go in via the security tree? > > > > If it hasn't been upstreamed yet, just make sure you put > > > > cc: stable@kernel.org > > > > In the signoffs of the patch you're sending upstream and the backport > > will occur automatically when the patch is finally upstreamed. > > Mimi didn't write or send the patch. Thanks for explaining it to everyone and upstreaming the patch. Mimi > This happily git-free maintainer simply goes in and edits the > changelog. I do this very very frequently. > > Here's what I currently have. I plan to send this to Rip Van Viro. > > > > > From: Dmitry Kasatkin <dmitry.kasatkin@intel.com> > Subject: vfs: increment iversion when a file is truncated > > When a file is truncated with truncate()/ftruncate() and then closed, > iversion is not updated. This patch uses ATTR_SIZE flag as an indication > to increment iversion. > > Mimi said: > > On fput(), i_version is used to detect and flag files that have changed > and need to be re-measured in the IMA measurement policy. When a file > is truncated with truncate()/ftruncate() and then closed, i_version is > not updated. As a result, although the file has changed, it will not be > re-measured and added to the IMA measurement list on subsequent access. > > Signed-off-by: Dmitry Kasatkin <dmitry.kasatkin@intel.com> > Acked-by: Mimi Zohar <zohar@us.ibm.com> > Cc: Al Viro <viro@zeniv.linux.org.uk> > Cc: <stable@vger.kernel.org> > Signed-off-by: Andrew Morton <akpm@linux-foundation.org> > --- > > fs/attr.c | 5 +++++ > 1 file changed, 5 insertions(+) > > diff -puN fs/attr.c~vfs-increment-iversion-when-a-file-is-truncated fs/attr.c > --- a/fs/attr.c~vfs-increment-iversion-when-a-file-is-truncated > +++ a/fs/attr.c > @@ -176,6 +176,11 @@ int notify_change(struct dentry * dentry > return -EPERM; > } > > + if ((ia_valid & ATTR_SIZE) && IS_I_VERSION(inode)) { > + if (attr->ia_size != inode->i_size) > + inode_inc_iversion(inode); > + } > + > if ((ia_valid & ATTR_MODE)) { > umode_t amode = attr->ia_mode; > /* Flag setting protected by i_mutex */ > _ > ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH v3 1/1] vfs: iversion truncate bug fix 2012-01-05 22:30 ` Andrew Morton 2012-01-05 23:09 ` Mimi Zohar @ 2012-01-13 10:13 ` Kasatkin, Dmitry 1 sibling, 0 replies; 18+ messages in thread From: Kasatkin, Dmitry @ 2012-01-13 10:13 UTC (permalink / raw) To: Andrew Morton Cc: James Bottomley, Mimi Zohar, Greg KH, linux-fsdevel, linux-security-module, viro, linux-kernel On Fri, Jan 6, 2012 at 12:30 AM, Andrew Morton <akpm@linux-foundation.org> wrote: > On Thu, 05 Jan 2012 10:39:41 -0800 > James Bottomley <James.Bottomley@HansenPartnership.com> wrote: > >> > > Please, go read Documentation/stable_kernel_rules.txt for how to >> > > properly submit patches to the stable kernel tree. The information here >> > > needs to be in the patch changelog itself, not in some random email >> > > thread that will get lost instantly into my email-archive-from-hell >> > > after I am done reading this. >> > > >> > > greg k-h >> > >> > Yes, I've read Documentation/stable_kernel_rules.txt and think this >> > patch meets the criteria for being backported. >> > >> > As far as I'm aware, this patch hasn't been upstreamed yet and is >> > waiting for someone, besides myself, to Ack it. Once Acked, either >> > Dmitry or I can send a pull request with an updated patch description. >> > Should this patch go in via the security tree? >> >> If it hasn't been upstreamed yet, just make sure you put >> >> cc: stable@kernel.org >> >> In the signoffs of the patch you're sending upstream and the backport >> will occur automatically when the patch is finally upstreamed. > > Mimi didn't write or send the patch. > > This happily git-free maintainer simply goes in and edits the > changelog. I do this very very frequently. > > > Here's what I currently have. I plan to send this to Rip Van Viro. > > Hello, Should I do anything about re-submitting this patch now? Or it went forward already? Thanks, Dmitry > > > From: Dmitry Kasatkin <dmitry.kasatkin@intel.com> > Subject: vfs: increment iversion when a file is truncated > > When a file is truncated with truncate()/ftruncate() and then closed, > iversion is not updated. This patch uses ATTR_SIZE flag as an indication > to increment iversion. > > Mimi said: > > On fput(), i_version is used to detect and flag files that have changed > and need to be re-measured in the IMA measurement policy. When a file > is truncated with truncate()/ftruncate() and then closed, i_version is > not updated. As a result, although the file has changed, it will not be > re-measured and added to the IMA measurement list on subsequent access. > > Signed-off-by: Dmitry Kasatkin <dmitry.kasatkin@intel.com> > Acked-by: Mimi Zohar <zohar@us.ibm.com> > Cc: Al Viro <viro@zeniv.linux.org.uk> > Cc: <stable@vger.kernel.org> > Signed-off-by: Andrew Morton <akpm@linux-foundation.org> > --- > > fs/attr.c | 5 +++++ > 1 file changed, 5 insertions(+) > > diff -puN fs/attr.c~vfs-increment-iversion-when-a-file-is-truncated fs/attr.c > --- a/fs/attr.c~vfs-increment-iversion-when-a-file-is-truncated > +++ a/fs/attr.c > @@ -176,6 +176,11 @@ int notify_change(struct dentry * dentry > return -EPERM; > } > > + if ((ia_valid & ATTR_SIZE) && IS_I_VERSION(inode)) { > + if (attr->ia_size != inode->i_size) > + inode_inc_iversion(inode); > + } > + > if ((ia_valid & ATTR_MODE)) { > umode_t amode = attr->ia_mode; > /* Flag setting protected by i_mutex */ > _ ^ permalink raw reply [flat|nested] 18+ messages in thread
end of thread, other threads:[~2012-01-13 10:14 UTC | newest] Thread overview: 18+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2011-12-22 10:54 [PATCH v3 1/1] vfs: iversion truncate bug fix Dmitry Kasatkin 2011-12-22 13:26 ` Mimi Zohar 2012-01-04 23:28 ` Andrew Morton 2012-01-05 0:33 ` Mimi Zohar 2012-01-05 0:46 ` Andrew Morton 2012-01-05 2:06 ` Greg KH 2012-01-05 4:17 ` Mimi Zohar 2012-01-05 4:53 ` Dave Chinner 2012-01-05 18:14 ` Christoph Hellwig 2012-01-05 19:49 ` Mimi Zohar 2012-01-05 21:36 ` J. Bruce Fields 2012-01-05 22:27 ` Mimi Zohar 2012-01-05 16:54 ` Greg KH 2012-01-05 17:19 ` Mimi Zohar 2012-01-05 18:39 ` James Bottomley 2012-01-05 22:30 ` Andrew Morton 2012-01-05 23:09 ` Mimi Zohar 2012-01-13 10:13 ` Kasatkin, Dmitry
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).