lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [thread-next>] [day] [month] [year] [list]
Date:	Fri, 16 Jan 2009 23:51:35 -0800 (PST)
From:	Paul Turner <pjt@...gle.com>
To:	linux-kernel@...r.kernel.org
cc:	adobriyan@...il.com
Subject: Migration of kernel interfaces to seq_files breaks pread() consumers


(Specifically) Several interfaces under /proc have been migrated to use 
seq_files.  This was previously observed to be a problem with VMware's 
reading of /proc/uptime.  We're now running into the same problem on  
/proc/<pid>/stat; we have many consumers performing preads on this 
interface which break under new kernels.

Reverting these migrations presents other problems and doesn't scale with 
everyones' pet dependencies over an abi that's been
broken :(

Part of the problem in implementing pread in seq_files is that we don't  
know know whether the read was issued by pread(2) or read(2).  It's not 
nice to shoehorn this information down the stack.  I've attached a 
skeleton patch which shows one way we could push it up (although something 
like a second f_pos would be necessary to make it maintain pread 
semantics against reads).

One advantage of this style of approach is that it doesn't break on 
partial record reads.  But it's a little gross at the same time.

Comments?  Ideas? 

Thanks,

- Paul

--
 fs/read_write.c    |   10 ++++++++++
 fs/seq_file.c      |    2 ++
 include/linux/fs.h |    2 ++
 3 files changed, 14 insertions(+), 0 deletions(-)

diff --git a/fs/read_write.c b/fs/read_write.c
index 2fc2980..744094a 100644
--- a/fs/read_write.c
+++ b/fs/read_write.c
@@ -407,6 +407,16 @@ asmlinkage ssize_t sys_pread64(unsigned int fd, char __user *buf,
 		ret = -ESPIPE;
 		if (file->f_mode & FMODE_PREAD)
 			ret = vfs_read(file, buf, count, &pos);
+		else if (file->f_mode & FMODE_SEQ_FILE) {
+			/*
+			 * We break the pread semantic and actually make it
+			 * seek, this prevents inconsistent record reads across
+			 * boundaries.
+			 */
+			vfs_llseek(file, pos, SEEK_SET);
+			ret = vfs_read(file, buf, count, &pos);
+			file_pos_write(file, pos);
+		}
 		fput_light(file, fput_needed);
 	}
 
diff --git a/fs/seq_file.c b/fs/seq_file.c
index 3f54dbd..f8c5379 100644
--- a/fs/seq_file.c
+++ b/fs/seq_file.c
@@ -50,6 +50,8 @@ int seq_open(struct file *file, const struct seq_operations *op)
 
 	/* SEQ files support lseek, but not pread/pwrite */
 	file->f_mode &= ~(FMODE_PREAD | FMODE_PWRITE);
+	file->f_mode |= FMODE_SEQ_FILE;
+
 	return 0;
 }
 EXPORT_SYMBOL(seq_open);
diff --git a/include/linux/fs.h b/include/linux/fs.h
index 5f7b912..c3b5916 100644
--- a/include/linux/fs.h
+++ b/include/linux/fs.h
@@ -76,6 +76,8 @@ extern int dir_notify_enable;
    behavior for cross-node execution/opening_for_writing of files */
 #define FMODE_EXEC	16
 
+#define FMODE_SEQ_FILE_PREAD	32
+
 #define RW_MASK		1
 #define RWA_MASK	2
 #define READ 0
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Powered by blists - more mailing lists