From: "Arkadiusz Kozdra (Arusekk)" <arek_koz@o2.pl>
To: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc: Linus Torvalds <torvalds@linux-foundation.org>,
Christoph Hellwig <hch@lst.de>,
Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
linux-fsdevel@vger.kernel.org,
"Arkadiusz Kozdra (Arusekk)" <arek_koz@o2.pl>,
Alexey Dobriyan <adobriyan@gmail.com>,
Al Viro <viro@zeniv.linux.org.uk>
Subject: [PATCH v3] proc: Use seq_read_iter for /proc/*/maps
Date: Tue, 4 May 2021 13:53:58 +0200 [thread overview]
Message-ID: <20210504115358.20741-1-arek_koz@o2.pl> (raw)
In-Reply-To: <YIqFcHj3O2t+JJak@kroah.com>
Since seq_read_iter looks mature enough to be used for /proc/<pid>/maps,
re-allow applications to perform zero-copy data forwarding from it.
Some executable-inspecting tools (e.g. pwntools) rely on patching entry
point instructions with minimal machine code that uses sendfile to read
/proc/self/maps to stdout. The sendfile call allows them to do it
without excessive allocations, which would change the mappings, and
therefore distort the information.
This is inspired by the series by Cristoph Hellwig (linked).
Link: https://lore.kernel.org/lkml/20201104082738.1054792-1-hch@lst.de/
Fixes: 36e2c7421f02 ("fs: don't allow splice read/write without explicit ops")
Cc: Alexey Dobriyan <adobriyan@gmail.com>
Cc: Al Viro <viro@zeniv.linux.org.uk>
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc: Christoph Hellwig <hch@lst.de>
Signed-off-by: Arkadiusz Kozdra (Arusekk) <arek_koz@o2.pl>
---
v3:
- Only commit message changed.
- Clarify what tools use this.
- Do not mention performance.
The average execution time of a patched static ELF outputting to a pipe
(the use case of pwntools inspecting mappings of an executable)
was varying both before and after ca. 3.43ms +-0.05ms (I decided that
the performance impact is not worth mentioning in the commit message).
I think that the change should probably marginally improve speed, but
it will most likely also affect the memory footprint and as such likely
minimally decrease power consumption (I suppose it would only be
measurable when the mappings description grows many pages long).
Speed might be more affected in pathological cases like a close-to-OOM
scenario, but I was unable to test this reliably.
I did my tests under qemu-system-x86_64 on a Ryzen 4500 host without
kvm, with default kernel config.
If someone wants to also test this feature of pwntools for themselves,
it can be used as follows and should print something other than None:
$ pip install pwntools
$ python3
>>> from pwn import *
>>> print(ELF("/bin/true").libc)
Sorry for the delay, but it took me much time to figure out some
low-overhead timing methods.
Does this change need selftests? It looks like it should never break
again if it only uses common code hopefully tested elsewhere.
fs/proc/task_mmu.c | 3 ++-
fs/proc/task_nommu.c | 3 ++-
2 files changed, 4 insertions(+), 2 deletions(-)
diff --git a/fs/proc/task_mmu.c b/fs/proc/task_mmu.c
index e862cab69583..06282294ddb8 100644
--- a/fs/proc/task_mmu.c
+++ b/fs/proc/task_mmu.c
@@ -351,7 +351,8 @@ static int pid_maps_open(struct inode *inode, struct file *file)
const struct file_operations proc_pid_maps_operations = {
.open = pid_maps_open,
- .read = seq_read,
+ .read_iter = seq_read_iter,
+ .splice_read = generic_file_splice_read,
.llseek = seq_lseek,
.release = proc_map_release,
};
diff --git a/fs/proc/task_nommu.c b/fs/proc/task_nommu.c
index a6d21fc0033c..e55e79fd0175 100644
--- a/fs/proc/task_nommu.c
+++ b/fs/proc/task_nommu.c
@@ -295,7 +295,8 @@ static int pid_maps_open(struct inode *inode, struct file *file)
const struct file_operations proc_pid_maps_operations = {
.open = pid_maps_open,
- .read = seq_read,
+ .read_iter = seq_read_iter,
+ .splice_read = generic_file_splice_read,
.llseek = seq_lseek,
.release = map_release,
};
--
2.31.1
next prev parent reply other threads:[~2021-05-04 11:56 UTC|newest]
Thread overview: 12+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-04-27 18:34 [PATCH] proc: Use seq_read_iter where possible Arkadiusz Kozdra (Arusekk)
2021-04-28 6:12 ` Christoph Hellwig
2021-04-28 13:02 ` Arusekk
2021-04-28 13:03 ` Christoph Hellwig
2021-04-28 17:46 ` Linus Torvalds
2021-04-29 10:05 ` [PATCH v2] proc: Use seq_read_iter for /proc/*/maps Arkadiusz Kozdra (Arusekk)
2021-04-29 10:07 ` Greg Kroah-Hartman
2021-05-04 11:53 ` Arkadiusz Kozdra (Arusekk) [this message]
2021-05-04 16:01 ` [PATCH v3] " Linus Torvalds
2021-05-04 20:23 ` Arusekk
2021-05-04 20:24 ` Linus Torvalds
2021-04-29 16:36 ` [PATCH v2] " Linus Torvalds
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20210504115358.20741-1-arek_koz@o2.pl \
--to=arek_koz@o2.pl \
--cc=adobriyan@gmail.com \
--cc=gregkh@linuxfoundation.org \
--cc=hch@lst.de \
--cc=linux-fsdevel@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=torvalds@linux-foundation.org \
--cc=viro@zeniv.linux.org.uk \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
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).