[PATCH v1 0/2] fs: fix and deduplicate whence handling in default_llseek()

[PATCH v1 0/2] fs: fix and deduplicate whence handling in default_llseek()

[Siddharth Chintamaneni]

This series fixes negative offset validation for SEEK_DATA and SEEK_HOLE
in default_llseek() and deduplicates its whence handling with
must_set_pos().

Patch 1 updates default_llseek() to read the inode size once via
i_size_read(inode) and compare (unsigned long long)offset >= eof for
SEEK_DATA and SEEK_HOLE so negative offsets fail with -ENXIO instead of
bypassing the range check.

Patch 2 replaces the open-coded switch (whence) block in
default_llseek() with a call to must_set_pos().

Siddharth Chintamaneni (2):
  fs: reject negative offsets for SEEK_DATA and SEEK_HOLE in
    default_llseek()
  fs: use must_set_pos() in default_llseek()

 fs/read_write.c | 48 ++++++++++++++----------------------------------
 1 file changed, 14 insertions(+), 34 deletions(-)


base-commit: 36aad21d50d1af3f6954bb0c988257a908202477
-- 
2.50.1 (Apple Git-155)

[PATCH v1 1/2] fs: reject negative offsets for SEEK_DATA and SEEK_HOLE in default_llseek()

[Siddharth Chintamaneni]

In default_llseek(), the range checks for SEEK_DATA and SEEK_HOLE compare
the signed loff_t offset directly against inode->i_size:

	if (offset >= inode->i_size) {
		retval = -ENXIO;
		goto out;
	}

When a negative offset is supplied to SEEK_HOLE (e.g. lseek(fd, -10,
SEEK_HOLE) on a sysctl file in /proc/sys), the signed comparison evaluates
to false and execution proceeds to:

	offset = inode->i_size;

This overwrites the negative offset with a non-negative value before the
final `if (offset >= 0 || unsigned_offsets(file))` check, causing
default_llseek() to update file->f_pos and return inode->i_size instead of
failing with -ENXIO. Similarly, for SEEK_DATA on a file with
FOP_UNSIGNED_OFFSET, a negative offset bypasses the range check and
succeeds via unsigned_offsets(file).

Read the inode size once via i_size_read(inode) after acquiring the inode
lock and compare (unsigned long long)offset >= eof for both SEEK_DATA and
SEEK_HOLE, matching must_set_pos().

Fixes: 982d816581ee ("fs: add SEEK_HOLE and SEEK_DATA flags")
Signed-off-by: Siddharth Chintamaneni <sidchintamaneni@gmail.com>
---
 fs/read_write.c | 10 ++++++----
 1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/fs/read_write.c b/fs/read_write.c
index 676728310899..86ce8cea10d7 100644
--- a/fs/read_write.c
+++ b/fs/read_write.c
@@ -333,13 +333,15 @@ loff_t default_llseek(struct file *file, loff_t offset, int whence)
 {
 	struct inode *inode = file_inode(file);
 	loff_t retval;
+	loff_t eof;
 
 	retval = inode_lock_killable(inode);
 	if (retval)
 		return retval;
+	eof = i_size_read(inode);
 	switch (whence) {
 		case SEEK_END:
-			offset += i_size_read(inode);
+			offset += eof;
 			break;
 		case SEEK_CUR:
 			if (offset == 0) {
@@ -354,7 +356,7 @@ loff_t default_llseek(struct file *file, loff_t offset, int whence)
 			 * long as offset isn't at the end of the file then the
 			 * offset is data.
 			 */
-			if (offset >= inode->i_size) {
+			if ((unsigned long long)offset >= eof) {
 				retval = -ENXIO;
 				goto out;
 			}
@@ -365,11 +367,11 @@ loff_t default_llseek(struct file *file, loff_t offset, int whence)
 			 * as long as offset isn't i_size or larger, return
 			 * i_size.
 			 */
-			if (offset >= inode->i_size) {
+			if ((unsigned long long)offset >= eof) {
 				retval = -ENXIO;
 				goto out;
 			}
-			offset = inode->i_size;
+			offset = eof;
 			break;
 	}
 	retval = -EINVAL;
-- 
2.50.1 (Apple Git-155)

[PATCH v1 2/2] fs: use must_set_pos() in default_llseek()

[Siddharth Chintamaneni]

default_llseek() open-codes the same whence switch for SEEK_END,
SEEK_CUR, SEEK_DATA, and SEEK_HOLE that must_set_pos() already implements
for generic_file_llseek_size() and generic_llseek_cookie().

Replace the open-coded switch (whence) block in default_llseek() with a
call to must_set_pos(file, &offset, whence, i_size_read(inode)).

Signed-off-by: Siddharth Chintamaneni <sidchintamaneni@gmail.com>
---
 fs/read_write.c | 50 ++++++++++++++-----------------------------------
 1 file changed, 14 insertions(+), 36 deletions(-)

diff --git a/fs/read_write.c b/fs/read_write.c
index 86ce8cea10d7..b15522cf96cd 100644
--- a/fs/read_write.c
+++ b/fs/read_write.c
@@ -333,47 +333,25 @@ loff_t default_llseek(struct file *file, loff_t offset, int whence)
 {
 	struct inode *inode = file_inode(file);
 	loff_t retval;
-	loff_t eof;
+	int ret;
 
 	retval = inode_lock_killable(inode);
 	if (retval)
 		return retval;
-	eof = i_size_read(inode);
-	switch (whence) {
-		case SEEK_END:
-			offset += eof;
-			break;
-		case SEEK_CUR:
-			if (offset == 0) {
-				retval = file->f_pos;
-				goto out;
-			}
-			offset += file->f_pos;
-			break;
-		case SEEK_DATA:
-			/*
-			 * In the generic case the entire file is data, so as
-			 * long as offset isn't at the end of the file then the
-			 * offset is data.
-			 */
-			if ((unsigned long long)offset >= eof) {
-				retval = -ENXIO;
-				goto out;
-			}
-			break;
-		case SEEK_HOLE:
-			/*
-			 * There is a virtual hole at the end of the file, so
-			 * as long as offset isn't i_size or larger, return
-			 * i_size.
-			 */
-			if ((unsigned long long)offset >= eof) {
-				retval = -ENXIO;
-				goto out;
-			}
-			offset = eof;
-			break;
+
+	ret = must_set_pos(file, &offset, whence, i_size_read(inode));
+	if (ret < 0) {
+		retval = ret;
+		goto out;
+	}
+	if (ret == 0) {
+		retval = offset;
+		goto out;
 	}
+
+	if (whence == SEEK_CUR)
+		offset += file->f_pos;
+
 	retval = -EINVAL;
 	if (offset >= 0 || unsigned_offsets(file)) {
 		if (offset != file->f_pos)
-- 
2.50.1 (Apple Git-155)