Skip to contentSkip to conversation

vfs: iversion truncate bug fix

PATCH In review Dmitry Kasatkin 8 participants 18 messages Dec 22, 2011 – Jan 13, 2012 VFS

lore
Share & tools
lore
#0 lore raw

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
index 7ee7ba4..b8f55c4 100644
@@ -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
Acked-by on v3 1/1
#1 lore raw
Quoted 5 lines · 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.

(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
index 7ee7ba4..b8f55c4 100644
@@ -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 */
#3 lore raw
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_SIZE flag as an indication to increment iversion.

Quoted 6 lines
Quoted 4 lines
Quoted 1 line

(Stable should be cc'ed on this patch.)

why?

Why backported? The IMA measurement list could be incomplete.

Mimi

#4 lore raw
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_SIZE flag as an indication to increment iversion.

Quoted 8 lines
Quoted 6 lines
Quoted 4 lines
Quoted 1 line

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

#5 lore raw
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_SIZE flag as an indication to increment iversion.

Quoted 8 lines
Quoted 6 lines
Quoted 4 lines
Quoted 1 line

(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 :)

#6 lore raw
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_SIZE flag as an indication to increment iversion.

Quoted 8 lines
Quoted 6 lines
Quoted 4 lines
Quoted 1 line

(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

#7 lore raw
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_SIZE flag as an indication to increment iversion.

Quoted 8 lines
Quoted 6 lines
Quoted 4 lines
Quoted 1 line

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

Signature · 2 lines
Dave Chinner
[email protected]
#8 lore raw
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 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.

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_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?

#9 lore raw
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 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.

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_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:

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

#10 lore raw
In reply to Christoph Hellwig
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 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.

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

Quoted 8 lines

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.

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

#11 lore raw
In reply to Christoph Hellwig
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 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.

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

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

#12 lore raw
In reply to Mimi Zohar
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_SIZE flag as an indication to increment iversion.

Quoted 8 lines
Quoted 6 lines
Quoted 4 lines
Quoted 1 line

(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_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

#13 lore raw
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_SIZE flag as an indication to increment iversion.

Quoted 8 lines
Quoted 6 lines
Quoted 4 lines
Quoted 1 line

(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_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?

Quoted 7 lines

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

#14 lore raw
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_SIZE flag as an indication to increment iversion.

Quoted 8 lines
Quoted 6 lines
Quoted 4 lines
Quoted 1 line

(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_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?

Quoted 7 lines

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

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

In the signoffs of the patch you're sending upstream and the backport will occur automatically when the patch is finally upstreamed.

James

Acked-by Mimi Zohar
#15 lore raw
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.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 7 lines

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?

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
diff -puN fs/attr.c~vfs-increment-iversion-when-a-file-is-truncated 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 */

_

#16 lore raw
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.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?

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 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(+)

diff -puN fs/attr.c~vfs-increment-iversion-when-a-file-is-truncated fs/attr.c

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 * 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 */

_

#17 lore raw
In reply to Andrew Morton
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.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 7 lines

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?

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 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(+)

diff -puN fs/attr.c~vfs-increment-iversion-when-a-file-is-truncated fs/attr.c

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