linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v2 0/8] fs: add some missing ctime updates
@ 2023-06-12 10:45 Jeff Layton
  2023-06-12 10:45 ` [PATCH v2 1/8] ibmvmc: update ctime in conjunction with mtime on write Jeff Layton
                   ` (8 more replies)
  0 siblings, 9 replies; 15+ messages in thread
From: Jeff Layton @ 2023-06-12 10:45 UTC (permalink / raw)
  To: Christian Brauner, Al Viro, Brad Warrum, Ritu Agarwal,
	Arnd Bergmann, Greg Kroah-Hartman, Ian Kent, Tigran A. Aivazian,
	Jeremy Kerr, Ard Biesheuvel, Namjae Jeon, Sungjong Seo,
	Steve French, Paulo Alcantara, Ronnie Sahlberg, Shyam Prasad N,
	Tom Talpey, John Johansen, Paul Moore, James Morris,
	Serge E. Hallyn, Ruihan Li, Sebastian Reichel, Alan Stern,
	Suren Baghdasaryan, Wolfram Sang, linux-kernel, linux-usb,
	autofs, linux-efi, linux-fsdevel, linux-cifs, samba-technical,
	apparmor, linux-security-module

v2:
- drop gfs2 patch as it involved (hidden) quota inode
- clarify patch descriptions to satisfy checkpatch.pl

While working on a patch series to change how we handle the ctime, I
found a number of places that update the mtime without a corresponding
ctime update.

While it's not spelled out explicitly in the POSIX spec, all of the
operations that update the mtime must also update the ctime. I've not
been able to find any counterexamples, in any case. Some of these
patches involve operations not covered by POSIX, but it's still a good
idea to update the ctime when updating the mtime.

Note that these are largely untested other than for compilation, so
please review carefully. These are a preliminary set for the upcoming
rework of how we handle the ctime.

None of these seem to be very crucial, but it would be nice if various
maintainers could pick these up for v6.5. Please let me know if you do,
or would rather I shepherd the patch upstream.

Jeff Layton (8):
  ibmvmc: update ctime in conjunction with mtime on write
  usb: update the ctime as well when updating mtime after an ioctl
  autofs: set ctime as well when mtime changes on a dir
  bfs: update ctime in addition to mtime when adding entries
  efivarfs: update ctime when mtime changes on a write
  exfat: ensure that ctime is updated whenever the mtime is
  apparmor: update ctime whenever the mtime changes on an inode
  cifs: update the ctime on a partial page write

 drivers/misc/ibmvmc.c             |  2 +-
 drivers/usb/core/devio.c          | 16 ++++++++--------
 fs/autofs/root.c                  |  6 +++---
 fs/bfs/dir.c                      |  2 +-
 fs/efivarfs/file.c                |  2 +-
 fs/exfat/namei.c                  |  8 ++++----
 fs/smb/client/file.c              |  2 +-
 security/apparmor/apparmorfs.c    |  7 +++++--
 security/apparmor/policy_unpack.c | 11 +++++++----
 9 files changed, 31 insertions(+), 25 deletions(-)

-- 
2.40.1


^ permalink raw reply	[flat|nested] 15+ messages in thread

* [PATCH v2 1/8] ibmvmc: update ctime in conjunction with mtime on write
  2023-06-12 10:45 [PATCH v2 0/8] fs: add some missing ctime updates Jeff Layton
@ 2023-06-12 10:45 ` Jeff Layton
  2023-06-12 10:45 ` [PATCH v2 2/8] usb: update the ctime as well when updating mtime after an ioctl Jeff Layton
                   ` (7 subsequent siblings)
  8 siblings, 0 replies; 15+ messages in thread
From: Jeff Layton @ 2023-06-12 10:45 UTC (permalink / raw)
  To: Christian Brauner, Al Viro, Brad Warrum, Ritu Agarwal,
	Arnd Bergmann, Greg Kroah-Hartman, Ian Kent, Tigran A. Aivazian,
	Jeremy Kerr, Ard Biesheuvel, Namjae Jeon, Sungjong Seo,
	Steve French, Paulo Alcantara, Ronnie Sahlberg, Shyam Prasad N,
	Tom Talpey, John Johansen, Paul Moore, James Morris,
	Serge E. Hallyn, Ruihan Li, Sebastian Reichel, Alan Stern,
	Suren Baghdasaryan, Wolfram Sang, linux-kernel, linux-usb,
	autofs, linux-efi, linux-fsdevel, linux-cifs, samba-technical,
	apparmor, linux-security-module

POSIX says:

"Upon successful completion, where nbyte is greater than 0, write()
 shall mark for update the last data modification and last file status
 change timestamps of the file..."

Signed-off-by: Jeff Layton <jlayton@kernel.org>
---
 drivers/misc/ibmvmc.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/misc/ibmvmc.c b/drivers/misc/ibmvmc.c
index cbaf6d35e854..d7c7f0305257 100644
--- a/drivers/misc/ibmvmc.c
+++ b/drivers/misc/ibmvmc.c
@@ -1124,7 +1124,7 @@ static ssize_t ibmvmc_write(struct file *file, const char *buffer,
 		goto out;
 
 	inode = file_inode(file);
-	inode->i_mtime = current_time(inode);
+	inode->i_mtime = inode->i_ctime = current_time(inode);
 	mark_inode_dirty(inode);
 
 	dev_dbg(adapter->dev, "write: file = 0x%lx, count = 0x%lx\n",
-- 
2.40.1


^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [PATCH v2 2/8] usb: update the ctime as well when updating mtime after an ioctl
  2023-06-12 10:45 [PATCH v2 0/8] fs: add some missing ctime updates Jeff Layton
  2023-06-12 10:45 ` [PATCH v2 1/8] ibmvmc: update ctime in conjunction with mtime on write Jeff Layton
@ 2023-06-12 10:45 ` Jeff Layton
  2023-06-12 10:45 ` [PATCH v2 3/8] autofs: set ctime as well when mtime changes on a dir Jeff Layton
                   ` (6 subsequent siblings)
  8 siblings, 0 replies; 15+ messages in thread
From: Jeff Layton @ 2023-06-12 10:45 UTC (permalink / raw)
  To: Christian Brauner, Al Viro, Brad Warrum, Ritu Agarwal,
	Arnd Bergmann, Greg Kroah-Hartman, Ian Kent, Tigran A. Aivazian,
	Jeremy Kerr, Ard Biesheuvel, Namjae Jeon, Sungjong Seo,
	Steve French, Paulo Alcantara, Ronnie Sahlberg, Shyam Prasad N,
	Tom Talpey, John Johansen, Paul Moore, James Morris,
	Serge E. Hallyn, Ruihan Li, Sebastian Reichel, Alan Stern,
	Suren Baghdasaryan, Wolfram Sang, linux-kernel, linux-usb,
	autofs, linux-efi, linux-fsdevel, linux-cifs, samba-technical,
	apparmor, linux-security-module

In general, POSIX requires that when the mtime is updated that the ctime
be updated as well. Add the missing timestamp updates to the usb ioctls.

Signed-off-by: Jeff Layton <jlayton@kernel.org>
---
 drivers/usb/core/devio.c | 16 ++++++++--------
 1 file changed, 8 insertions(+), 8 deletions(-)

diff --git a/drivers/usb/core/devio.c b/drivers/usb/core/devio.c
index fcf68818e999..1268d313a8df 100644
--- a/drivers/usb/core/devio.c
+++ b/drivers/usb/core/devio.c
@@ -2640,21 +2640,21 @@ static long usbdev_do_ioctl(struct file *file, unsigned int cmd,
 		snoop(&dev->dev, "%s: CONTROL\n", __func__);
 		ret = proc_control(ps, p);
 		if (ret >= 0)
-			inode->i_mtime = current_time(inode);
+			inode->i_mtime = inode->i_ctime = current_time(inode);
 		break;
 
 	case USBDEVFS_BULK:
 		snoop(&dev->dev, "%s: BULK\n", __func__);
 		ret = proc_bulk(ps, p);
 		if (ret >= 0)
-			inode->i_mtime = current_time(inode);
+			inode->i_mtime = inode->i_ctime = current_time(inode);
 		break;
 
 	case USBDEVFS_RESETEP:
 		snoop(&dev->dev, "%s: RESETEP\n", __func__);
 		ret = proc_resetep(ps, p);
 		if (ret >= 0)
-			inode->i_mtime = current_time(inode);
+			inode->i_mtime = inode->i_ctime = current_time(inode);
 		break;
 
 	case USBDEVFS_RESET:
@@ -2666,7 +2666,7 @@ static long usbdev_do_ioctl(struct file *file, unsigned int cmd,
 		snoop(&dev->dev, "%s: CLEAR_HALT\n", __func__);
 		ret = proc_clearhalt(ps, p);
 		if (ret >= 0)
-			inode->i_mtime = current_time(inode);
+			inode->i_mtime = inode->i_ctime = current_time(inode);
 		break;
 
 	case USBDEVFS_GETDRIVER:
@@ -2693,7 +2693,7 @@ static long usbdev_do_ioctl(struct file *file, unsigned int cmd,
 		snoop(&dev->dev, "%s: SUBMITURB\n", __func__);
 		ret = proc_submiturb(ps, p);
 		if (ret >= 0)
-			inode->i_mtime = current_time(inode);
+			inode->i_mtime = inode->i_ctime = current_time(inode);
 		break;
 
 #ifdef CONFIG_COMPAT
@@ -2701,14 +2701,14 @@ static long usbdev_do_ioctl(struct file *file, unsigned int cmd,
 		snoop(&dev->dev, "%s: CONTROL32\n", __func__);
 		ret = proc_control_compat(ps, p);
 		if (ret >= 0)
-			inode->i_mtime = current_time(inode);
+			inode->i_mtime = inode->i_ctime = current_time(inode);
 		break;
 
 	case USBDEVFS_BULK32:
 		snoop(&dev->dev, "%s: BULK32\n", __func__);
 		ret = proc_bulk_compat(ps, p);
 		if (ret >= 0)
-			inode->i_mtime = current_time(inode);
+			inode->i_mtime = inode->i_ctime = current_time(inode);
 		break;
 
 	case USBDEVFS_DISCSIGNAL32:
@@ -2720,7 +2720,7 @@ static long usbdev_do_ioctl(struct file *file, unsigned int cmd,
 		snoop(&dev->dev, "%s: SUBMITURB32\n", __func__);
 		ret = proc_submiturb_compat(ps, p);
 		if (ret >= 0)
-			inode->i_mtime = current_time(inode);
+			inode->i_mtime = inode->i_ctime = current_time(inode);
 		break;
 
 	case USBDEVFS_IOCTL32:
-- 
2.40.1


^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [PATCH v2 3/8] autofs: set ctime as well when mtime changes on a dir
  2023-06-12 10:45 [PATCH v2 0/8] fs: add some missing ctime updates Jeff Layton
  2023-06-12 10:45 ` [PATCH v2 1/8] ibmvmc: update ctime in conjunction with mtime on write Jeff Layton
  2023-06-12 10:45 ` [PATCH v2 2/8] usb: update the ctime as well when updating mtime after an ioctl Jeff Layton
@ 2023-06-12 10:45 ` Jeff Layton
  2023-06-13  7:16   ` Ian Kent
  2023-06-14  8:30   ` (subset) " Christian Brauner
  2023-06-12 10:45 ` [PATCH v2 4/8] bfs: update ctime in addition to mtime when adding entries Jeff Layton
                   ` (5 subsequent siblings)
  8 siblings, 2 replies; 15+ messages in thread
From: Jeff Layton @ 2023-06-12 10:45 UTC (permalink / raw)
  To: Christian Brauner, Al Viro, Brad Warrum, Ritu Agarwal,
	Arnd Bergmann, Greg Kroah-Hartman, Ian Kent, Tigran A. Aivazian,
	Jeremy Kerr, Ard Biesheuvel, Namjae Jeon, Sungjong Seo,
	Steve French, Paulo Alcantara, Ronnie Sahlberg, Shyam Prasad N,
	Tom Talpey, John Johansen, Paul Moore, James Morris,
	Serge E. Hallyn, Ruihan Li, Sebastian Reichel, Alan Stern,
	Suren Baghdasaryan, Wolfram Sang, linux-kernel, linux-usb,
	autofs, linux-efi, linux-fsdevel, linux-cifs, samba-technical,
	apparmor, linux-security-module

When adding entries to a directory, POSIX generally requires that the
ctime also be updated alongside the mtime.

Signed-off-by: Jeff Layton <jlayton@kernel.org>
---
 fs/autofs/root.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/fs/autofs/root.c b/fs/autofs/root.c
index 6baf90b08e0e..93046c9dc461 100644
--- a/fs/autofs/root.c
+++ b/fs/autofs/root.c
@@ -600,7 +600,7 @@ static int autofs_dir_symlink(struct mnt_idmap *idmap,
 	p_ino = autofs_dentry_ino(dentry->d_parent);
 	p_ino->count++;
 
-	dir->i_mtime = current_time(dir);
+	dir->i_mtime = dir->i_ctime = current_time(dir);
 
 	return 0;
 }
@@ -633,7 +633,7 @@ static int autofs_dir_unlink(struct inode *dir, struct dentry *dentry)
 	d_inode(dentry)->i_size = 0;
 	clear_nlink(d_inode(dentry));
 
-	dir->i_mtime = current_time(dir);
+	dir->i_mtime = dir->i_ctime = current_time(dir);
 
 	spin_lock(&sbi->lookup_lock);
 	__autofs_add_expiring(dentry);
@@ -749,7 +749,7 @@ static int autofs_dir_mkdir(struct mnt_idmap *idmap,
 	p_ino = autofs_dentry_ino(dentry->d_parent);
 	p_ino->count++;
 	inc_nlink(dir);
-	dir->i_mtime = current_time(dir);
+	dir->i_mtime = dir->i_ctime = current_time(dir);
 
 	return 0;
 }
-- 
2.40.1


^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [PATCH v2 4/8] bfs: update ctime in addition to mtime when adding entries
  2023-06-12 10:45 [PATCH v2 0/8] fs: add some missing ctime updates Jeff Layton
                   ` (2 preceding siblings ...)
  2023-06-12 10:45 ` [PATCH v2 3/8] autofs: set ctime as well when mtime changes on a dir Jeff Layton
@ 2023-06-12 10:45 ` Jeff Layton
  2023-06-12 10:45 ` [PATCH v2 5/8] efivarfs: update ctime when mtime changes on a write Jeff Layton
                   ` (4 subsequent siblings)
  8 siblings, 0 replies; 15+ messages in thread
From: Jeff Layton @ 2023-06-12 10:45 UTC (permalink / raw)
  To: Christian Brauner, Al Viro, Brad Warrum, Ritu Agarwal,
	Arnd Bergmann, Greg Kroah-Hartman, Ian Kent, Tigran A. Aivazian,
	Jeremy Kerr, Ard Biesheuvel, Namjae Jeon, Sungjong Seo,
	Steve French, Paulo Alcantara, Ronnie Sahlberg, Shyam Prasad N,
	Tom Talpey, John Johansen, Paul Moore, James Morris,
	Serge E. Hallyn, Ruihan Li, Sebastian Reichel, Alan Stern,
	Suren Baghdasaryan, Wolfram Sang, linux-kernel, linux-usb,
	autofs, linux-efi, linux-fsdevel, linux-cifs, samba-technical,
	apparmor, linux-security-module

When adding entries to a directory, POSIX generally requires that the
ctime be updated alongside the mtime.

Signed-off-by: Jeff Layton <jlayton@kernel.org>
---
 fs/bfs/dir.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/fs/bfs/dir.c b/fs/bfs/dir.c
index 040d5140e426..d2e8a2a56b05 100644
--- a/fs/bfs/dir.c
+++ b/fs/bfs/dir.c
@@ -294,7 +294,7 @@ static int bfs_add_entry(struct inode *dir, const struct qstr *child, int ino)
 					dir->i_size += BFS_DIRENT_SIZE;
 					dir->i_ctime = current_time(dir);
 				}
-				dir->i_mtime = current_time(dir);
+				dir->i_mtime = dir->i_ctime = current_time(dir);
 				mark_inode_dirty(dir);
 				de->ino = cpu_to_le16((u16)ino);
 				for (i = 0; i < BFS_NAMELEN; i++)
-- 
2.40.1


^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [PATCH v2 5/8] efivarfs: update ctime when mtime changes on a write
  2023-06-12 10:45 [PATCH v2 0/8] fs: add some missing ctime updates Jeff Layton
                   ` (3 preceding siblings ...)
  2023-06-12 10:45 ` [PATCH v2 4/8] bfs: update ctime in addition to mtime when adding entries Jeff Layton
@ 2023-06-12 10:45 ` Jeff Layton
  2023-06-12 10:45 ` [PATCH v2 6/8] exfat: ensure that ctime is updated whenever the mtime is Jeff Layton
                   ` (3 subsequent siblings)
  8 siblings, 0 replies; 15+ messages in thread
From: Jeff Layton @ 2023-06-12 10:45 UTC (permalink / raw)
  To: Christian Brauner, Al Viro, Brad Warrum, Ritu Agarwal,
	Arnd Bergmann, Greg Kroah-Hartman, Ian Kent, Tigran A. Aivazian,
	Jeremy Kerr, Ard Biesheuvel, Namjae Jeon, Sungjong Seo,
	Steve French, Paulo Alcantara, Ronnie Sahlberg, Shyam Prasad N,
	Tom Talpey, John Johansen, Paul Moore, James Morris,
	Serge E. Hallyn, Ruihan Li, Sebastian Reichel, Alan Stern,
	Suren Baghdasaryan, Wolfram Sang, linux-kernel, linux-usb,
	autofs, linux-efi, linux-fsdevel, linux-cifs, samba-technical,
	apparmor, linux-security-module

POSIX says:

"Upon successful completion, where nbyte is greater than 0, write()
 shall mark for update the last data modification and last file status
 change timestamps of the file..."

Add the missing ctime update.

Signed-off-by: Jeff Layton <jlayton@kernel.org>
---
 fs/efivarfs/file.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/fs/efivarfs/file.c b/fs/efivarfs/file.c
index d57ee15874f9..375576111dc3 100644
--- a/fs/efivarfs/file.c
+++ b/fs/efivarfs/file.c
@@ -51,7 +51,7 @@ static ssize_t efivarfs_file_write(struct file *file,
 	} else {
 		inode_lock(inode);
 		i_size_write(inode, datasize + sizeof(attributes));
-		inode->i_mtime = current_time(inode);
+		inode->i_mtime = inode->i_ctime = current_time(inode);
 		inode_unlock(inode);
 	}
 
-- 
2.40.1


^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [PATCH v2 6/8] exfat: ensure that ctime is updated whenever the mtime is
  2023-06-12 10:45 [PATCH v2 0/8] fs: add some missing ctime updates Jeff Layton
                   ` (4 preceding siblings ...)
  2023-06-12 10:45 ` [PATCH v2 5/8] efivarfs: update ctime when mtime changes on a write Jeff Layton
@ 2023-06-12 10:45 ` Jeff Layton
  2023-06-12 10:45 ` [PATCH v2 7/8] apparmor: update ctime whenever the mtime changes on an inode Jeff Layton
                   ` (2 subsequent siblings)
  8 siblings, 0 replies; 15+ messages in thread
From: Jeff Layton @ 2023-06-12 10:45 UTC (permalink / raw)
  To: Christian Brauner, Al Viro, Brad Warrum, Ritu Agarwal,
	Arnd Bergmann, Greg Kroah-Hartman, Ian Kent, Tigran A. Aivazian,
	Jeremy Kerr, Ard Biesheuvel, Namjae Jeon, Sungjong Seo,
	Steve French, Paulo Alcantara, Ronnie Sahlberg, Shyam Prasad N,
	Tom Talpey, John Johansen, Paul Moore, James Morris,
	Serge E. Hallyn, Ruihan Li, Sebastian Reichel, Alan Stern,
	Suren Baghdasaryan, Wolfram Sang, linux-kernel, linux-usb,
	autofs, linux-efi, linux-fsdevel, linux-cifs, samba-technical,
	apparmor, linux-security-module

When removing entries from a directory, the ctime must also be updated
alongside the mtime.

Signed-off-by: Jeff Layton <jlayton@kernel.org>
---
 fs/exfat/namei.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/fs/exfat/namei.c b/fs/exfat/namei.c
index e0ff9d156f6f..d9b46fa36bff 100644
--- a/fs/exfat/namei.c
+++ b/fs/exfat/namei.c
@@ -817,7 +817,7 @@ static int exfat_unlink(struct inode *dir, struct dentry *dentry)
 	ei->dir.dir = DIR_DELETED;
 
 	inode_inc_iversion(dir);
-	dir->i_mtime = dir->i_atime = current_time(dir);
+	dir->i_mtime = dir->i_atime = dir->i_ctime = current_time(dir);
 	exfat_truncate_atime(&dir->i_atime);
 	if (IS_DIRSYNC(dir))
 		exfat_sync_inode(dir);
@@ -825,7 +825,7 @@ static int exfat_unlink(struct inode *dir, struct dentry *dentry)
 		mark_inode_dirty(dir);
 
 	clear_nlink(inode);
-	inode->i_mtime = inode->i_atime = current_time(inode);
+	inode->i_mtime = inode->i_atime = inode->i_ctime = current_time(inode);
 	exfat_truncate_atime(&inode->i_atime);
 	exfat_unhash_inode(inode);
 	exfat_d_version_set(dentry, inode_query_iversion(dir));
@@ -979,7 +979,7 @@ static int exfat_rmdir(struct inode *dir, struct dentry *dentry)
 	ei->dir.dir = DIR_DELETED;
 
 	inode_inc_iversion(dir);
-	dir->i_mtime = dir->i_atime = current_time(dir);
+	dir->i_mtime = dir->i_atime = dir->i_ctime = current_time(dir);
 	exfat_truncate_atime(&dir->i_atime);
 	if (IS_DIRSYNC(dir))
 		exfat_sync_inode(dir);
@@ -988,7 +988,7 @@ static int exfat_rmdir(struct inode *dir, struct dentry *dentry)
 	drop_nlink(dir);
 
 	clear_nlink(inode);
-	inode->i_mtime = inode->i_atime = current_time(inode);
+	inode->i_mtime = inode->i_atime = inode->i_ctime = current_time(inode);
 	exfat_truncate_atime(&inode->i_atime);
 	exfat_unhash_inode(inode);
 	exfat_d_version_set(dentry, inode_query_iversion(dir));
-- 
2.40.1


^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [PATCH v2 7/8] apparmor: update ctime whenever the mtime changes on an inode
  2023-06-12 10:45 [PATCH v2 0/8] fs: add some missing ctime updates Jeff Layton
                   ` (5 preceding siblings ...)
  2023-06-12 10:45 ` [PATCH v2 6/8] exfat: ensure that ctime is updated whenever the mtime is Jeff Layton
@ 2023-06-12 10:45 ` Jeff Layton
  2023-06-12 10:45 ` [PATCH v2 8/8] cifs: update the ctime on a partial page write Jeff Layton
  2023-08-30  0:08 ` [PATCH v2 0/8] fs: add some missing ctime updates Al Viro
  8 siblings, 0 replies; 15+ messages in thread
From: Jeff Layton @ 2023-06-12 10:45 UTC (permalink / raw)
  To: Christian Brauner, Al Viro, Brad Warrum, Ritu Agarwal,
	Arnd Bergmann, Greg Kroah-Hartman, Ian Kent, Tigran A. Aivazian,
	Jeremy Kerr, Ard Biesheuvel, Namjae Jeon, Sungjong Seo,
	Steve French, Paulo Alcantara, Ronnie Sahlberg, Shyam Prasad N,
	Tom Talpey, John Johansen, Paul Moore, James Morris,
	Serge E. Hallyn, Ruihan Li, Sebastian Reichel, Alan Stern,
	Suren Baghdasaryan, Wolfram Sang, linux-kernel, linux-usb,
	autofs, linux-efi, linux-fsdevel, linux-cifs, samba-technical,
	apparmor, linux-security-module

In general, when updating the mtime on an inode, one must also update
the ctime. Add the missing ctime updates.

Signed-off-by: Jeff Layton <jlayton@kernel.org>
---
 security/apparmor/apparmorfs.c    |  7 +++++--
 security/apparmor/policy_unpack.c | 11 +++++++----
 2 files changed, 12 insertions(+), 6 deletions(-)

diff --git a/security/apparmor/apparmorfs.c b/security/apparmor/apparmorfs.c
index db7a51acf9db..c06053718836 100644
--- a/security/apparmor/apparmorfs.c
+++ b/security/apparmor/apparmorfs.c
@@ -1554,8 +1554,11 @@ void __aafs_profile_migrate_dents(struct aa_profile *old,
 
 	for (i = 0; i < AAFS_PROF_SIZEOF; i++) {
 		new->dents[i] = old->dents[i];
-		if (new->dents[i])
-			new->dents[i]->d_inode->i_mtime = current_time(new->dents[i]->d_inode);
+		if (new->dents[i]) {
+			struct inode *inode = d_inode(new->dents[i]);
+
+			inode->i_mtime = inode->i_ctime = current_time(inode);
+		}
 		old->dents[i] = NULL;
 	}
 }
diff --git a/security/apparmor/policy_unpack.c b/security/apparmor/policy_unpack.c
index cf2ceec40b28..48a97c1800b9 100644
--- a/security/apparmor/policy_unpack.c
+++ b/security/apparmor/policy_unpack.c
@@ -86,10 +86,13 @@ void __aa_loaddata_update(struct aa_loaddata *data, long revision)
 
 	data->revision = revision;
 	if ((data->dents[AAFS_LOADDATA_REVISION])) {
-		d_inode(data->dents[AAFS_LOADDATA_DIR])->i_mtime =
-			current_time(d_inode(data->dents[AAFS_LOADDATA_DIR]));
-		d_inode(data->dents[AAFS_LOADDATA_REVISION])->i_mtime =
-			current_time(d_inode(data->dents[AAFS_LOADDATA_REVISION]));
+		struct inode *inode;
+
+		inode = d_inode(data->dents[AAFS_LOADDATA_DIR]);
+		inode->i_mtime = inode->i_ctime = current_time(inode);
+
+		inode = d_inode(data->dents[AAFS_LOADDATA_REVISION]);
+		inode->i_mtime = inode->i_ctime = current_time(inode);
 	}
 }
 
-- 
2.40.1


^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [PATCH v2 8/8] cifs: update the ctime on a partial page write
  2023-06-12 10:45 [PATCH v2 0/8] fs: add some missing ctime updates Jeff Layton
                   ` (6 preceding siblings ...)
  2023-06-12 10:45 ` [PATCH v2 7/8] apparmor: update ctime whenever the mtime changes on an inode Jeff Layton
@ 2023-06-12 10:45 ` Jeff Layton
  2023-06-12 13:41   ` Tom Talpey
  2023-08-30  0:08 ` [PATCH v2 0/8] fs: add some missing ctime updates Al Viro
  8 siblings, 1 reply; 15+ messages in thread
From: Jeff Layton @ 2023-06-12 10:45 UTC (permalink / raw)
  To: Christian Brauner, Al Viro, Brad Warrum, Ritu Agarwal,
	Arnd Bergmann, Greg Kroah-Hartman, Ian Kent, Tigran A. Aivazian,
	Jeremy Kerr, Ard Biesheuvel, Namjae Jeon, Sungjong Seo,
	Steve French, Paulo Alcantara, Ronnie Sahlberg, Shyam Prasad N,
	Tom Talpey, John Johansen, Paul Moore, James Morris,
	Serge E. Hallyn, Ruihan Li, Sebastian Reichel, Alan Stern,
	Suren Baghdasaryan, Wolfram Sang, linux-kernel, linux-usb,
	autofs, linux-efi, linux-fsdevel, linux-cifs, samba-technical,
	apparmor, linux-security-module

POSIX says:

    "Upon successful completion, where nbyte is greater than 0, write()
     shall mark for update the last data modification and last file status
     change timestamps of the file..."

Add the missing ctime update.

Signed-off-by: Jeff Layton <jlayton@kernel.org>
---
 fs/smb/client/file.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/fs/smb/client/file.c b/fs/smb/client/file.c
index df88b8c04d03..a00038a326cf 100644
--- a/fs/smb/client/file.c
+++ b/fs/smb/client/file.c
@@ -2596,7 +2596,7 @@ static int cifs_partialpagewrite(struct page *page, unsigned from, unsigned to)
 					   write_data, to - from, &offset);
 		cifsFileInfo_put(open_file);
 		/* Does mm or vfs already set times? */
-		inode->i_atime = inode->i_mtime = current_time(inode);
+		inode->i_atime = inode->i_mtime = inode->i_ctime = current_time(inode);
 		if ((bytes_written > 0) && (offset))
 			rc = 0;
 		else if (bytes_written < 0)
-- 
2.40.1


^ permalink raw reply related	[flat|nested] 15+ messages in thread

* Re: [PATCH v2 8/8] cifs: update the ctime on a partial page write
  2023-06-12 10:45 ` [PATCH v2 8/8] cifs: update the ctime on a partial page write Jeff Layton
@ 2023-06-12 13:41   ` Tom Talpey
  2023-06-12 13:51     ` Jeff Layton
  0 siblings, 1 reply; 15+ messages in thread
From: Tom Talpey @ 2023-06-12 13:41 UTC (permalink / raw)
  To: Jeff Layton, Christian Brauner, Al Viro, Brad Warrum,
	Ritu Agarwal, Arnd Bergmann, Greg Kroah-Hartman, Ian Kent,
	Tigran A. Aivazian, Jeremy Kerr, Ard Biesheuvel, Namjae Jeon,
	Sungjong Seo, Steve French, Paulo Alcantara, Ronnie Sahlberg,
	Shyam Prasad N, John Johansen, Paul Moore, James Morris,
	Serge E. Hallyn, Ruihan Li, Sebastian Reichel, Alan Stern,
	Suren Baghdasaryan, Wolfram Sang, linux-kernel, linux-usb,
	autofs, linux-efi, linux-fsdevel, linux-cifs, samba-technical,
	apparmor, linux-security-module

On 6/12/2023 6:45 AM, Jeff Layton wrote:
> POSIX says:
> 
>      "Upon successful completion, where nbyte is greater than 0, write()
>       shall mark for update the last data modification and last file status
>       change timestamps of the file..."
> 
> Add the missing ctime update.
> 
> Signed-off-by: Jeff Layton <jlayton@kernel.org>
> ---
>   fs/smb/client/file.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/fs/smb/client/file.c b/fs/smb/client/file.c
> index df88b8c04d03..a00038a326cf 100644
> --- a/fs/smb/client/file.c
> +++ b/fs/smb/client/file.c
> @@ -2596,7 +2596,7 @@ static int cifs_partialpagewrite(struct page *page, unsigned from, unsigned to)
>   					   write_data, to - from, &offset);
>   		cifsFileInfo_put(open_file);
>   		/* Does mm or vfs already set times? */
> -		inode->i_atime = inode->i_mtime = current_time(inode);
> +		inode->i_atime = inode->i_mtime = inode->i_ctime = current_time(inode);

Question. It appears that roughly half the filesystems in this series
don't touch the i_atime in this case. And the other half do. Which is
correct? Did they incorrectly set i_atime instead of i_ctime?

Tom.

>   		if ((bytes_written > 0) && (offset))
>   			rc = 0;
>   		else if (bytes_written < 0)

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH v2 8/8] cifs: update the ctime on a partial page write
  2023-06-12 13:41   ` Tom Talpey
@ 2023-06-12 13:51     ` Jeff Layton
  0 siblings, 0 replies; 15+ messages in thread
From: Jeff Layton @ 2023-06-12 13:51 UTC (permalink / raw)
  To: Tom Talpey, Christian Brauner, Al Viro, Brad Warrum,
	Ritu Agarwal, Arnd Bergmann, Greg Kroah-Hartman, Ian Kent,
	Tigran A. Aivazian, Jeremy Kerr, Ard Biesheuvel, Namjae Jeon,
	Sungjong Seo, Steve French, Paulo Alcantara, Ronnie Sahlberg,
	Shyam Prasad N, John Johansen, Paul Moore, James Morris,
	Serge E. Hallyn, Ruihan Li, Sebastian Reichel, Alan Stern,
	Suren Baghdasaryan, Wolfram Sang, linux-kernel, linux-usb,
	autofs, linux-efi, linux-fsdevel, linux-cifs, samba-technical,
	apparmor, linux-security-module

On Mon, 2023-06-12 at 09:41 -0400, Tom Talpey wrote:
> On 6/12/2023 6:45 AM, Jeff Layton wrote:
> > POSIX says:
> > 
> >      "Upon successful completion, where nbyte is greater than 0, write()
> >       shall mark for update the last data modification and last file status
> >       change timestamps of the file..."
> > 
> > Add the missing ctime update.
> > 
> > Signed-off-by: Jeff Layton <jlayton@kernel.org>
> > ---
> >   fs/smb/client/file.c | 2 +-
> >   1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/fs/smb/client/file.c b/fs/smb/client/file.c
> > index df88b8c04d03..a00038a326cf 100644
> > --- a/fs/smb/client/file.c
> > +++ b/fs/smb/client/file.c
> > @@ -2596,7 +2596,7 @@ static int cifs_partialpagewrite(struct page *page, unsigned from, unsigned to)
> >   					   write_data, to - from, &offset);
> >   		cifsFileInfo_put(open_file);
> >   		/* Does mm or vfs already set times? */
> > -		inode->i_atime = inode->i_mtime = current_time(inode);
> > +		inode->i_atime = inode->i_mtime = inode->i_ctime = current_time(inode);
> 
> Question. It appears that roughly half the filesystems in this series
> don't touch the i_atime in this case. And the other half do. Which is
> correct? Did they incorrectly set i_atime instead of i_ctime?
> 

I noticed that too, and with this set, I decided to not make any atime
changes since I wasn't sure.

I think the answer to your question is "it depends". atime is supposed
to be updated on reads, not writes, but sometimes a write requires a RMW
cycle of some flavor so one can imagine that in some cases we'd need to
update all three.

In this case, I'm not sure that updating any of these times is the right
thing to do. This is called from ->launder_folio, so the syscall that
issued the write is long gone and we're in writeback here.

With NFS, we generally leave timestamp updates to the server. Should any
of these timestamps be updated by the (SMB1) client here?
-- 
Jeff Layton <jlayton@kernel.org>

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH v2 3/8] autofs: set ctime as well when mtime changes on a dir
  2023-06-12 10:45 ` [PATCH v2 3/8] autofs: set ctime as well when mtime changes on a dir Jeff Layton
@ 2023-06-13  7:16   ` Ian Kent
  2023-06-14  8:30   ` (subset) " Christian Brauner
  1 sibling, 0 replies; 15+ messages in thread
From: Ian Kent @ 2023-06-13  7:16 UTC (permalink / raw)
  To: Jeff Layton, Christian Brauner, Al Viro, Brad Warrum,
	Ritu Agarwal, Arnd Bergmann, Greg Kroah-Hartman,
	Tigran A. Aivazian, Jeremy Kerr, Ard Biesheuvel, Namjae Jeon,
	Sungjong Seo, Steve French, Paulo Alcantara, Ronnie Sahlberg,
	Shyam Prasad N, Tom Talpey, John Johansen, Paul Moore,
	James Morris, Serge E. Hallyn, Ruihan Li, Sebastian Reichel,
	Alan Stern, Suren Baghdasaryan, Wolfram Sang, linux-kernel,
	linux-usb, autofs, linux-efi, linux-fsdevel, linux-cifs,
	samba-technical, apparmor, linux-security-module

On 12/6/23 18:45, Jeff Layton wrote:
> When adding entries to a directory, POSIX generally requires that the
> ctime also be updated alongside the mtime.
>
> Signed-off-by: Jeff Layton <jlayton@kernel.org>

Acked-by: Ian Kent <raven@themaw.net>


> ---
>   fs/autofs/root.c | 6 +++---
>   1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/fs/autofs/root.c b/fs/autofs/root.c
> index 6baf90b08e0e..93046c9dc461 100644
> --- a/fs/autofs/root.c
> +++ b/fs/autofs/root.c
> @@ -600,7 +600,7 @@ static int autofs_dir_symlink(struct mnt_idmap *idmap,
>   	p_ino = autofs_dentry_ino(dentry->d_parent);
>   	p_ino->count++;
>   
> -	dir->i_mtime = current_time(dir);
> +	dir->i_mtime = dir->i_ctime = current_time(dir);
>   
>   	return 0;
>   }
> @@ -633,7 +633,7 @@ static int autofs_dir_unlink(struct inode *dir, struct dentry *dentry)
>   	d_inode(dentry)->i_size = 0;
>   	clear_nlink(d_inode(dentry));
>   
> -	dir->i_mtime = current_time(dir);
> +	dir->i_mtime = dir->i_ctime = current_time(dir);
>   
>   	spin_lock(&sbi->lookup_lock);
>   	__autofs_add_expiring(dentry);
> @@ -749,7 +749,7 @@ static int autofs_dir_mkdir(struct mnt_idmap *idmap,
>   	p_ino = autofs_dentry_ino(dentry->d_parent);
>   	p_ino->count++;
>   	inc_nlink(dir);
> -	dir->i_mtime = current_time(dir);
> +	dir->i_mtime = dir->i_ctime = current_time(dir);
>   
>   	return 0;
>   }

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: (subset) [PATCH v2 3/8] autofs: set ctime as well when mtime changes on a dir
  2023-06-12 10:45 ` [PATCH v2 3/8] autofs: set ctime as well when mtime changes on a dir Jeff Layton
  2023-06-13  7:16   ` Ian Kent
@ 2023-06-14  8:30   ` Christian Brauner
  2023-06-15 10:14     ` Ian Kent
  1 sibling, 1 reply; 15+ messages in thread
From: Christian Brauner @ 2023-06-14  8:30 UTC (permalink / raw)
  To: Jeff Layton
  Cc: Christian Brauner, Al Viro, Brad Warrum, Ritu Agarwal,
	Arnd Bergmann, Greg Kroah-Hartman, Ian Kent, Tigran A. Aivazian,
	Jeremy Kerr, Ard Biesheuvel, Namjae Jeon, Sungjong Seo,
	Steve French, Paulo Alcantara, Ronnie Sahlberg, Shyam Prasad N,
	Tom Talpey, John Johansen, Paul Moore, James Morris,
	Serge E. Hallyn, Ruihan Li, Sebastian Reichel, Alan Stern,
	Suren Baghdasaryan, Wolfram Sang, linux-kernel, linux-usb,
	autofs, linux-efi, linux-fsdevel, linux-cifs, samba-technical,
	apparmor, linux-security-module

On Mon, 12 Jun 2023 06:45:19 -0400, Jeff Layton wrote:
> When adding entries to a directory, POSIX generally requires that the
> ctime also be updated alongside the mtime.
> 
> 

Can't find a tree for this patch, so picking this patch up unless told otherwise.

---

Applied to the vfs.misc branch of the vfs/vfs.git tree.
Patches in the vfs.misc branch should appear in linux-next soon.

Please report any outstanding bugs that were missed during review in a
new review to the original patch series allowing us to drop it.

It's encouraged to provide Acked-bys and Reviewed-bys even though the
patch has now been applied. If possible patch trailers will be updated.

tree:   https://git.kernel.org/pub/scm/linux/kernel/git/vfs/vfs.git
branch: vfs.misc

[3/8] autofs: set ctime as well when mtime changes on a dir
      https://git.kernel.org/vfs/vfs/c/9b37b3342a98

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: (subset) [PATCH v2 3/8] autofs: set ctime as well when mtime changes on a dir
  2023-06-14  8:30   ` (subset) " Christian Brauner
@ 2023-06-15 10:14     ` Ian Kent
  0 siblings, 0 replies; 15+ messages in thread
From: Ian Kent @ 2023-06-15 10:14 UTC (permalink / raw)
  To: Christian Brauner, Jeff Layton
  Cc: Al Viro, Brad Warrum, Ritu Agarwal, Arnd Bergmann,
	Greg Kroah-Hartman, Tigran A. Aivazian, Jeremy Kerr,
	Ard Biesheuvel, Namjae Jeon, Sungjong Seo, Steve French,
	Paulo Alcantara, Ronnie Sahlberg, Shyam Prasad N, Tom Talpey,
	John Johansen, Paul Moore, James Morris, Serge E. Hallyn,
	Ruihan Li, Sebastian Reichel, Alan Stern, Suren Baghdasaryan,
	Wolfram Sang, linux-kernel, linux-usb, autofs, linux-efi,
	linux-fsdevel, linux-cifs, samba-technical, apparmor,
	linux-security-module

On 14/6/23 16:30, Christian Brauner wrote:
> On Mon, 12 Jun 2023 06:45:19 -0400, Jeff Layton wrote:
>> When adding entries to a directory, POSIX generally requires that the
>> ctime also be updated alongside the mtime.
>>
>>
> Can't find a tree for this patch, so picking this patch up unless told otherwise.

There's relatively few changes to autofs and Linus asked me to send

changes via. Al or Andrew so there's no point in maintaining a tree

anyway.


Ian

>
> ---
>
> Applied to the vfs.misc branch of the vfs/vfs.git tree.
> Patches in the vfs.misc branch should appear in linux-next soon.
>
> Please report any outstanding bugs that were missed during review in a
> new review to the original patch series allowing us to drop it.
>
> It's encouraged to provide Acked-bys and Reviewed-bys even though the
> patch has now been applied. If possible patch trailers will be updated.
>
> tree:   https://git.kernel.org/pub/scm/linux/kernel/git/vfs/vfs.git
> branch: vfs.misc
>
> [3/8] autofs: set ctime as well when mtime changes on a dir
>        https://git.kernel.org/vfs/vfs/c/9b37b3342a98

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH v2 0/8] fs: add some missing ctime updates
  2023-06-12 10:45 [PATCH v2 0/8] fs: add some missing ctime updates Jeff Layton
                   ` (7 preceding siblings ...)
  2023-06-12 10:45 ` [PATCH v2 8/8] cifs: update the ctime on a partial page write Jeff Layton
@ 2023-08-30  0:08 ` Al Viro
  8 siblings, 0 replies; 15+ messages in thread
From: Al Viro @ 2023-08-30  0:08 UTC (permalink / raw)
  To: Jeff Layton
  Cc: Christian Brauner, Brad Warrum, Ritu Agarwal, Arnd Bergmann,
	Greg Kroah-Hartman, Ian Kent, Tigran A. Aivazian, Jeremy Kerr,
	Ard Biesheuvel, Namjae Jeon, Sungjong Seo, Steve French,
	Paulo Alcantara, Ronnie Sahlberg, Shyam Prasad N, Tom Talpey,
	John Johansen, Paul Moore, James Morris, Serge E. Hallyn,
	Ruihan Li, Sebastian Reichel, Alan Stern, Suren Baghdasaryan,
	Wolfram Sang, linux-kernel, linux-usb, autofs, linux-efi,
	linux-fsdevel, linux-cifs, samba-technical, apparmor,
	linux-security-module

On Mon, Jun 12, 2023 at 06:45:16AM -0400, Jeff Layton wrote:
> Jeff Layton (8):
>   ibmvmc: update ctime in conjunction with mtime on write
>   usb: update the ctime as well when updating mtime after an ioctl
>   autofs: set ctime as well when mtime changes on a dir
>   bfs: update ctime in addition to mtime when adding entries
>   efivarfs: update ctime when mtime changes on a write
>   exfat: ensure that ctime is updated whenever the mtime is
>   apparmor: update ctime whenever the mtime changes on an inode
>   cifs: update the ctime on a partial page write
> 
>  drivers/misc/ibmvmc.c             |  2 +-
>  drivers/usb/core/devio.c          | 16 ++++++++--------
>  fs/autofs/root.c                  |  6 +++---
>  fs/bfs/dir.c                      |  2 +-
>  fs/efivarfs/file.c                |  2 +-
>  fs/exfat/namei.c                  |  8 ++++----
>  fs/smb/client/file.c              |  2 +-
>  security/apparmor/apparmorfs.c    |  7 +++++--
>  security/apparmor/policy_unpack.c | 11 +++++++----
>  9 files changed, 31 insertions(+), 25 deletions(-)

As a possible followup (again, apologies for being MIA for months):
touch_cmtime(inode)...

^ permalink raw reply	[flat|nested] 15+ messages in thread

end of thread, other threads:[~2023-08-30  0:11 UTC | newest]

Thread overview: 15+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-06-12 10:45 [PATCH v2 0/8] fs: add some missing ctime updates Jeff Layton
2023-06-12 10:45 ` [PATCH v2 1/8] ibmvmc: update ctime in conjunction with mtime on write Jeff Layton
2023-06-12 10:45 ` [PATCH v2 2/8] usb: update the ctime as well when updating mtime after an ioctl Jeff Layton
2023-06-12 10:45 ` [PATCH v2 3/8] autofs: set ctime as well when mtime changes on a dir Jeff Layton
2023-06-13  7:16   ` Ian Kent
2023-06-14  8:30   ` (subset) " Christian Brauner
2023-06-15 10:14     ` Ian Kent
2023-06-12 10:45 ` [PATCH v2 4/8] bfs: update ctime in addition to mtime when adding entries Jeff Layton
2023-06-12 10:45 ` [PATCH v2 5/8] efivarfs: update ctime when mtime changes on a write Jeff Layton
2023-06-12 10:45 ` [PATCH v2 6/8] exfat: ensure that ctime is updated whenever the mtime is Jeff Layton
2023-06-12 10:45 ` [PATCH v2 7/8] apparmor: update ctime whenever the mtime changes on an inode Jeff Layton
2023-06-12 10:45 ` [PATCH v2 8/8] cifs: update the ctime on a partial page write Jeff Layton
2023-06-12 13:41   ` Tom Talpey
2023-06-12 13:51     ` Jeff Layton
2023-08-30  0:08 ` [PATCH v2 0/8] fs: add some missing ctime updates Al Viro

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