vfs: iversion truncate bug fix
PATCH v3 1/1 vfs: iversion truncate bug fix
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.
--- fs/attr.c | 5 +++++ 1 files changed, 5 insertions(+), 0 deletions(-)
fs/attr.c +5 -0
@@ -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 */
Signature · 1 line
1.7.4.1
Quoted 5 lines · Dmitry Kasatkin
When a file is truncated with truncate()/ftruncate() and then closed, iversion is not updated. This patch uses
ATTR_SIZEflag as an indication to increment iversion.Signed-off-by Dmitry Kasatkin <[email protected]>
(Stable should be cc'ed on this patch.)
thanks,
Mimi
Quoted 20 lines
--- fs/attr.c | 5 +++++ 1 files changed, 5 insertions(+), 0 deletions(-)fs/attr.c +5
-0@@ -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 */
Quoted 9 lines · On Thu, 22 Dec 2011 08:26:30 -0500 Mimi Zohar <[email protected]> wrote:
Quoted 5 lines · Dmitry Kasatkin
When a file is truncated with truncate()/ftruncate() and then closed, iversion is not updated. This patch uses
ATTR_SIZEflag as an indication to increment iversion.Signed-off-by Dmitry Kasatkin <[email protected]>Acked-by Mimi Zohar <[email protected]>(Stable should be cc'ed on this patch.)
why?
Quoted 8 lines · Andrew Morton
Quoted 5 lines · On Thu, 22 Dec 2011 08:26:30 -0500 Mimi Zohar <[email protected]> wrote:
Quoted 4 lines · Dmitry Kasatkin
When a file is truncated with truncate()/ftruncate() and then closed, iversion is not updated. This patch uses
ATTR_SIZEflag as an indication to increment iversion.
Quoted 6 lines
Quoted 4 lines
Quoted 1 line
Signed-off-by Dmitry Kasatkin <[email protected]>Acked-by Mimi Zohar <[email protected]>(Stable should be cc'ed on this patch.)
why?
Why backported? The IMA measurement list could be incomplete.
Mimi
Quoted 9 lines · On Wed, 04 Jan 2012 19:33:49 -0500 Mimi Zohar <[email protected]> wrote:
Quoted 8 lines · Andrew Morton
Quoted 5 lines · On Thu, 22 Dec 2011 08:26:30 -0500 Mimi Zohar <[email protected]> wrote:
Quoted 4 lines · Dmitry Kasatkin
When a file is truncated with truncate()/ftruncate() and then closed, iversion is not updated. This patch uses
ATTR_SIZEflag as an indication to increment iversion.
Quoted 8 lines
Quoted 6 lines
Quoted 4 lines
Quoted 1 line
Signed-off-by Dmitry Kasatkin <[email protected]>Acked-by Mimi Zohar <[email protected]>(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.
Quoted 1 line
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.
Quoted 26 lines · Andrew Morton
Quoted 9 lines · On Wed, 04 Jan 2012 19:33:49 -0500 Mimi Zohar <[email protected]> wrote:
Quoted 8 lines · Andrew Morton
Quoted 5 lines · On Thu, 22 Dec 2011 08:26:30 -0500 Mimi Zohar <[email protected]> wrote:
Quoted 4 lines · Dmitry Kasatkin
When a file is truncated with truncate()/ftruncate() and then closed, iversion is not updated. This patch uses
ATTR_SIZEflag as an indication to increment iversion.Quoted 8 lines
Quoted 6 lines
Quoted 4 lines
Quoted 1 line
Signed-off-by Dmitry Kasatkin <[email protected]>Acked-by Mimi Zohar <[email protected]>(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.
Quoted 4 lines
Quoted 1 line
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 :)
Quoted 27 lines · Greg KH
Quoted 26 lines · Andrew Morton
Quoted 9 lines · On Wed, 04 Jan 2012 19:33:49 -0500 Mimi Zohar <[email protected]> wrote:
Quoted 8 lines · Andrew Morton
Quoted 5 lines · On Thu, 22 Dec 2011 08:26:30 -0500 Mimi Zohar <[email protected]> wrote:
Quoted 4 lines · Dmitry Kasatkin
When a file is truncated with truncate()/ftruncate() and then closed, iversion is not updated. This patch uses
ATTR_SIZEflag as an indication to increment iversion.Quoted 8 lines
Quoted 6 lines
Quoted 4 lines
Quoted 1 line
Signed-off-by Dmitry Kasatkin <[email protected]>Acked-by Mimi Zohar <[email protected]>(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.
Quoted 6 lines
Quoted 4 lines
Quoted 1 line
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
Quoted 33 lines · Mimi Zohar
Quoted 32 lines · Greg KH
Quoted 30 lines · Andrew Morton
Quoted 9 lines · On Wed, 04 Jan 2012 19:33:49 -0500 Mimi Zohar <[email protected]> wrote:
Quoted 8 lines · Andrew Morton
Quoted 5 lines · On Thu, 22 Dec 2011 08:26:30 -0500 Mimi Zohar <[email protected]> wrote:
Quoted 4 lines · Dmitry Kasatkin
When a file is truncated with truncate()/ftruncate() and then closed, iversion is not updated. This patch uses
ATTR_SIZEflag as an indication to increment iversion.Quoted 8 lines
Quoted 6 lines
Quoted 4 lines
Quoted 1 line
Signed-off-by Dmitry Kasatkin <[email protected]>Acked-by Mimi Zohar <[email protected]>(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.
Quoted 1 line
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.
Quoted 7 lines
Quoted 1 line
Nope, I don't either :)
On
fput(),i_versionis 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_versionis 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.
Signature · 2 lines
Dave Chinner [email protected]
Quoted 5 lines · Dave Chinner
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 theMS_I_VERSIONso thatinode_inc_versionis only called for ext4 inodes when timestamps change.
And even ext4 only does it when using the non-default "i_version" mount option.
Quoted 8 lines
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_versionwon't change....Perhaps bumping
i_versionin__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?
Quoted 31 lines · Christoph Hellwig
Quoted 5 lines · Dave Chinner
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 theMS_I_VERSIONso thatinode_inc_versionis only called for ext4 inodes when timestamps change.And even ext4 only does it when using the non-default "
i_version" mount option.Quoted 8 lines
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_versionwon't change....Perhaps bumping
i_versionin__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_versionupdates in there.That assumes we figure out a coherent way to do it - except for the conditional abuse in
file_updates_timesit's currently entirely under fs control. So the best way to fix it would be to:
Quoted 8 lines
- 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
Quoted 27 lines · Christoph Hellwig
Quoted 5 lines · Dave Chinner
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 theMS_I_VERSIONso thatinode_inc_versionis only called for ext4 inodes when timestamps change.And even ext4 only does it when using the non-default "
i_version" mount option.Quoted 8 lines
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_versionwon't change....Perhaps bumping
i_versionin__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_versionupdates in there.
Quoted 8 lines
That assumes we figure out a coherent way to do it - except for the conditional abuse in
file_updates_timesit'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.
Quoted 8 lines
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 [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Quoted 39 lines · Mimi Zohar
Quoted 38 lines · Christoph Hellwig
Quoted 5 lines · Dave Chinner
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 theMS_I_VERSIONso thatinode_inc_versionis only called for ext4 inodes when timestamps change.And even ext4 only does it when using the non-default "
i_version" mount option.Quoted 8 lines
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_versionwon't change....Perhaps bumping
i_versionin__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_versionupdates in there.That assumes we figure out a coherent way to do it - except for the conditional abuse in
file_updates_timesit'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.
Quoted 7 lines
Quoted 1 line
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
Quoted 33 lines · Mimi Zohar
Quoted 32 lines · Greg KH
Quoted 30 lines · Andrew Morton
Quoted 9 lines · On Wed, 04 Jan 2012 19:33:49 -0500 Mimi Zohar <[email protected]> wrote:
Quoted 8 lines · Andrew Morton
Quoted 5 lines · On Thu, 22 Dec 2011 08:26:30 -0500 Mimi Zohar <[email protected]> wrote:
Quoted 4 lines · Dmitry Kasatkin
When a file is truncated with truncate()/ftruncate() and then closed, iversion is not updated. This patch uses
ATTR_SIZEflag as an indication to increment iversion.Quoted 8 lines
Quoted 6 lines
Quoted 4 lines
Quoted 1 line
Signed-off-by Dmitry Kasatkin <[email protected]>Acked-by Mimi Zohar <[email protected]>(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.
Quoted 1 line
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.
Quoted 7 lines
Quoted 1 line
Nope, I don't either :)
On
fput(),i_versionis 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_versionis 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
Quoted 44 lines · Greg KH
Quoted 33 lines · Mimi Zohar
Quoted 32 lines · Greg KH
Quoted 30 lines · Andrew Morton
Quoted 9 lines · On Wed, 04 Jan 2012 19:33:49 -0500 Mimi Zohar <[email protected]> wrote:
Quoted 8 lines · Andrew Morton
Quoted 5 lines · On Thu, 22 Dec 2011 08:26:30 -0500 Mimi Zohar <[email protected]> wrote:
Quoted 4 lines · Dmitry Kasatkin
When a file is truncated with truncate()/ftruncate() and then closed, iversion is not updated. This patch uses
ATTR_SIZEflag as an indication to increment iversion.Quoted 8 lines
Quoted 6 lines
Quoted 4 lines
Quoted 1 line
Signed-off-by Dmitry Kasatkin <[email protected]>Acked-by Mimi Zohar <[email protected]>(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.
Quoted 1 line
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.
Quoted 7 lines
Quoted 1 line
Nope, I don't either :)
On
fput(),i_versionis 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_versionis 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?
Quoted 7 lines
Please, go read
Documentation/stable_kernel_rules.txtfor 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
Quoted 53 lines · Mimi Zohar
Quoted 44 lines · Greg KH
Quoted 33 lines · Mimi Zohar
Quoted 32 lines · Greg KH
Quoted 30 lines · Andrew Morton
Quoted 9 lines · On Wed, 04 Jan 2012 19:33:49 -0500 Mimi Zohar <[email protected]> wrote:
Quoted 8 lines · Andrew Morton
Quoted 5 lines · On Thu, 22 Dec 2011 08:26:30 -0500 Mimi Zohar <[email protected]> wrote:
Quoted 4 lines · Dmitry Kasatkin
When a file is truncated with truncate()/ftruncate() and then closed, iversion is not updated. This patch uses
ATTR_SIZEflag as an indication to increment iversion.Quoted 8 lines
Quoted 6 lines
Quoted 4 lines
Quoted 1 line
Signed-off-by Dmitry Kasatkin <[email protected]>Acked-by Mimi Zohar <[email protected]>(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.
Quoted 1 line
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.
Quoted 7 lines
Quoted 1 line
Nope, I don't either :)
On
fput(),i_versionis 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_versionis 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?
Quoted 7 lines
Please, go read
Documentation/stable_kernel_rules.txtfor 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
Quoted 7 lines
Yes, I've read
Documentation/stable_kernel_rules.txtand 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
In the signoffs of the patch you're sending upstream and the backport will occur automatically when the patch is finally upstreamed.
James
Andrew Morton
Acked-by Mimi ZoharQuoted 16 lines · On Thu, 05 Jan 2012 10:39:41 -0800 James Bottomley…
Quoted 8 lines
Quoted 7 lines
Please, go read
Documentation/stable_kernel_rules.txtfor 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
Quoted 7 lines
Yes, I've read
Documentation/stable_kernel_rules.txtand 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?
Quoted 6 lines
If it hasn't been upstreamed yet, just make sure you put
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 <[email protected]>
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.
---
fs/attr.c | 5 +++++ 1 file changed, 5 insertions(+)
fs/attr.c renamed from fs/attr.c~vfs-increment-iversion-when-a-file-is-truncated +5 -0
@@ -176,6 +176,11 @@ int notify_change(struct dentry * dentryreturn -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 */
_
Quoted 19 lines · Andrew Morton
Quoted 16 lines · On Thu, 05 Jan 2012 10:39:41 -0800 James Bottomley…
Quoted 15 lines
Quoted 7 lines
Please, go read
Documentation/stable_kernel_rules.txtfor 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.txtand 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?
Quoted 8 lines
Quoted 6 lines
If it hasn't been upstreamed yet, just make sure you put
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
Quoted 50 lines
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 <[email protected]>
Subject: vfs: increment iversion when a file is truncatedWhen a file is truncated with truncate()/ftruncate() and then closed, iversion is not updated. This patch uses
ATTR_SIZEflag as an indication to increment iversion.Mimi said:
On
fput(),i_versionis 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_versionis 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 <[email protected]>Acked-by Mimi Zohar <[email protected]>Cc Al Viro <[email protected]>Cc <[email protected]>Signed-off-by Andrew Morton <[email protected]>---
fs/attr.c | 5 +++++ 1 file changed, 5 insertions(+)diff -puN
fs/attr.c~vfs-increment-iversion-when-a-file-is-truncatedfs/attr.cfs/attr.c renamed from fs/attr.c~vfs-increment-iversion-when-a-file-is-truncated +5
-0@@ -176,6 +176,11 @@ int notify_change(struct dentry * dentryreturn -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 */ _
Quoted 26 lines · Andrew Morton
Quoted 16 lines · On Thu, 05 Jan 2012 10:39:41 -0800 James Bottomley…
Quoted 8 lines
Quoted 7 lines
Please, go read
Documentation/stable_kernel_rules.txtfor 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
Quoted 7 lines
Yes, I've read
Documentation/stable_kernel_rules.txtand 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?
Quoted 6 lines
If it hasn't been upstreamed yet, just make sure you put
In the signoffs of the patch you're sending upstream and the backport will occur automatically when the patch is finally upstreamed.
Quoted 9 lines
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
Quoted 42 lines
From: Dmitry Kasatkin <[email protected]>
Subject: vfs: increment iversion when a file is truncatedWhen a file is truncated with truncate()/ftruncate() and then closed, iversion is not updated. This patch uses
ATTR_SIZEflag as an indication to increment iversion.Mimi said:
On
fput(),i_versionis 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_versionis 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 <[email protected]>Acked-by Mimi Zohar <[email protected]>Cc Al Viro <[email protected]>Cc <[email protected]>Signed-off-by Andrew Morton <[email protected]>---
fs/attr.c | 5 +++++
1 file changed, 5 insertions(+)diff -puN
fs/attr.c~vfs-increment-iversion-when-a-file-is-truncatedfs/attr.cfs/attr.c renamed from fs/attr.c~vfs-increment-iversion-when-a-file-is-truncated +5
-0@@ -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 */
Quoted 1 line
_
--
To unsubscribe from this list: send the line "unsubscribe linux-security-module" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html