fs/namei: Don't update atime when some errors occur in get_link

Message ID 20231205071733.334474-1-gehao@kylinos.cn
State New
Headers
Series fs/namei: Don't update atime when some errors occur in get_link |

Commit Message

Hao Ge Dec. 5, 2023, 7:17 a.m. UTC
  Perhaps we have some errors occur(like security),then we don't update
atime,because we didn't actually access it

Signed-off-by: Hao Ge <gehao@kylinos.cn>
---
 fs/namei.c | 19 ++++++++++---------
 1 file changed, 10 insertions(+), 9 deletions(-)
  

Comments

Christian Brauner Dec. 5, 2023, 11:06 a.m. UTC | #1
On Tue, Dec 05, 2023 at 03:17:33PM +0800, Hao Ge wrote:
> Perhaps we have some errors occur(like security),then we don't update
> atime,because we didn't actually access it
> 
> Signed-off-by: Hao Ge <gehao@kylinos.cn>
> ---

We didn't follow the link but we accessed it. I guess it's not completey
clear what's correct here so I'd just leave it as is.
  
Hao Ge Dec. 6, 2023, 9:02 a.m. UTC | #2
> On Dec 5, 2023, at 19:07, Christian Brauner <brauner@kernel.org> wrote:
> 
> On Tue, Dec 05, 2023 at 03:17:33PM +0800, Hao Ge wrote:
>> Perhaps we have some errors occur(like security),then we don't update
>> atime,because we didn't actually access it
>> 
>> Signed-off-by: Hao Ge <gehao@kylinos.cn>
>> ---
> 
> We didn't follow the link but we accessed it. I guess it's not completey
> clear what's correct here so I'd just leave it as is.
Hi brauner
Thank your for your reply.
I just thought of a situation that user access a link failed due to some error(like permission issue),maybe report some error to user, actually user don’t get anything,but atime still update.
Maybe your are right,after all,we still tried to visit.
Thanks
Best Regards 
Hao
  

Patch

diff --git a/fs/namei.c b/fs/namei.c
index 71c13b2990b4..033d36d5c1c5 100644
--- a/fs/namei.c
+++ b/fs/namei.c
@@ -1779,15 +1779,6 @@  static const char *pick_link(struct nameidata *nd, struct path *link,
 			unlikely(link->mnt->mnt_flags & MNT_NOSYMFOLLOW))
 		return ERR_PTR(-ELOOP);
 
-	if (!(nd->flags & LOOKUP_RCU)) {
-		touch_atime(&last->link);
-		cond_resched();
-	} else if (atime_needs_update(&last->link, inode)) {
-		if (!try_to_unlazy(nd))
-			return ERR_PTR(-ECHILD);
-		touch_atime(&last->link);
-	}
-
 	error = security_inode_follow_link(link->dentry, inode,
 					   nd->flags & LOOKUP_RCU);
 	if (unlikely(error))
@@ -1810,6 +1801,16 @@  static const char *pick_link(struct nameidata *nd, struct path *link,
 		if (IS_ERR(res))
 			return res;
 	}
+
+	if (!(nd->flags & LOOKUP_RCU)) {
+		touch_atime(&last->link);
+		cond_resched();
+	} else if (atime_needs_update(&last->link, inode)) {
+		if (!try_to_unlazy(nd))
+			return ERR_PTR(-ECHILD);
+		touch_atime(&last->link);
+	}
+
 	if (*res == '/') {
 		error = nd_jump_root(nd);
 		if (unlikely(error))