Skip to contentSkip to conversation

fs: reject U64_MAX as last_mnt_id in listmount()

PATCH In review Tao Cui 2 participants 2 messages Sep 7

Subsystems (2)
Stable branch VFS
lore
Share & tools
lore

Tao Cui

author
#0 lore raw

PATCH 1/1 fs: reject U64_MAX as last_mnt_id in listmount()

From: Tao Cui <[email protected]>

listmount() uses mnt_id_req::param as a pagination cursor: on forward iteration do_listmount() starts from the first mount with an id strictly greater than last_mnt_id. The sanity check in prepare_klistmount() only rejects ids in the range [1, MNT_UNIQUE_ID_OFFSET]; U64_MAX passes the check, and incrementing it wraps back to 0, so mnt_find_id_at() restarts from the leftmost mount in the namespace every time.

A caller paging with last_mnt_id set to U64_MAX therefore always gets the same first batch of ids returned and can never advance to the end of the list:

# base: every call returns the same first batch listmount(param=U64_MAX) = 8 ids: 2147483886 2147483888 ... # patched:
listmount(param=U64_MAX) = -1 EINVAL

Reject U64_MAX up front together with the other invalid ids.

Fixes b4c2bea8ceaa ("add listmount(2) syscall")
Cc [email protected]
Signed-off-by Tao Cui <[email protected]>
---
 fs/namespace.c | 3 +++
 1 file changed, 3 insertions(+)
fs/namespace.c +3 -0
index 8d4009cfbf5b..cb95b855f78b 100644
@@ -6108,6 +6108,9 @@ static inline int prepare_klistmount(struct klistmount *kls, struct mnt_id_req *
 	/* The first valid unique mount id is MNT_UNIQUE_ID_OFFSET + 1. */
 	if (last_mnt_id != 0 && last_mnt_id <= MNT_UNIQUE_ID_OFFSET)
 		return -EINVAL;
+	/* U64_MAX would wrap to 0 and restart the iteration. */
+	if (last_mnt_id == U64_MAX)
+		return -EINVAL;
 
 	kls->last_mnt_id = last_mnt_id;
Signature · 1 line
2.43.0
reviewer
#1 lore raw
Quoted 18 lines · Tao Cui

From: Tao Cui <[email protected]>

listmount() uses mnt_id_req::param as a pagination cursor: on forward iteration do_listmount() starts from the first mount with an id strictly greater than last_mnt_id. The sanity check in prepare_klistmount() only rejects ids in the range [1, MNT_UNIQUE_ID_OFFSET]; U64_MAX passes the check, and incrementing it wraps back to 0, so mnt_find_id_at() restarts from the leftmost mount in the namespace every time.

A caller paging with last_mnt_id set to U64_MAX therefore always gets the same first batch of ids returned and can never advance to the end of the list:

# base: every call returns the same first batch listmount(param=U64_MAX) = 8 ids: 2147483886 2147483888 ... # patched:
listmount(param=U64_MAX) = -1 EINVAL

Quoted 5 lines

Reject U64_MAX up front together with the other invalid ids.

Fixes b4c2bea8ceaa ("add listmount(2) syscall")
Cc [email protected]
Signed-off-by Tao Cui <[email protected]>

This looks OK, but I think we should reject U64_MAX only if LISTMOUNT_REVERSE flag is not set? With this flag specifying U64_MAX actually makes sense and it is likely used by people?

Honza

Quoted 21 lines
---
 fs/namespace.c | 3 +++
 1 file changed, 3 insertions(+)
fs/namespace.c +3 -0
index 8d4009cfbf5b..cb95b855f78b 100644
@@ -6108,6 +6108,9 @@ static inline int prepare_klistmount(struct klistmount *kls, struct mnt_id_req *
 	/* The first valid unique mount id is MNT_UNIQUE_ID_OFFSET + 1. */
 	if (last_mnt_id != 0 && last_mnt_id <= MNT_UNIQUE_ID_OFFSET)
 		return -EINVAL;
+	/* U64_MAX would wrap to 0 and restart the iteration. */
+	if (last_mnt_id == U64_MAX)
+		return -EINVAL;
 
 	kls->last_mnt_id = last_mnt_id;
Signature · 1 line
2.43.0
Signature · 2 lines
Jan Kara <[email protected]>
SUSE Labs, CR