| 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
| ||
|
Message-Id: <20220801205039.2755281-1-haoluo@google.com>
Date: Mon, 1 Aug 2022 13:50:39 -0700
From: Hao Luo <haoluo@...gle.com>
To: linux-kernel@...r.kernel.org, bpf@...r.kernel.org
Cc: Alexei Starovoitov <ast@...nel.org>,
Andrii Nakryiko <andrii@...nel.org>,
Daniel Borkmann <daniel@...earbox.net>,
Martin KaFai Lau <martin.lau@...ux.dev>,
Song Liu <song@...nel.org>, Yonghong Song <yhs@...com>,
KP Singh <kpsingh@...nel.org>,
John Fastabend <john.fastabend@...il.com>,
Stanislav Fomichev <sdf@...gle.com>,
Jiri Olsa <jolsa@...nel.org>, Hao Luo <haoluo@...gle.com>
Subject: [PATCH bpf-next v1] bpf, iter: clean up bpf_seq_read().
Refactor bpf_seq_read() by extracting some common logic into helper
functions. I hope this makes bpf_seq_read() more readable. This is
a refactoring patch, so no behavior change is expected.
Signed-off-by: Hao Luo <haoluo@...gle.com>
---
kernel/bpf/bpf_iter.c | 156 +++++++++++++++++++++++++-----------------
1 file changed, 93 insertions(+), 63 deletions(-)
diff --git a/kernel/bpf/bpf_iter.c b/kernel/bpf/bpf_iter.c
index 7e8fd49406f6..39b5b647fdb7 100644
--- a/kernel/bpf/bpf_iter.c
+++ b/kernel/bpf/bpf_iter.c
@@ -77,6 +77,83 @@ static bool bpf_iter_support_resched(struct seq_file *seq)
return iter_priv->tinfo->reg_info->feature & BPF_ITER_RESCHED;
}
+/* do_copy_to_user, copies seq->buf at offset seq->from to userspace and
+ * updates corresponding fields in seq. It returns -EFAULT if any error
+ * happens. The actual number of bytes copied is returned via the argument
+ * 'copied'.
+ */
+static int do_copy_to_user(struct seq_file *seq, char __user *buf, size_t size,
+ size_t *copied)
+{
+ size_t n;
+
+ n = min(seq->count, size);
+ if (copy_to_user(buf, seq->buf + seq->from, n))
+ return -EFAULT;
+
+ seq->count -= n;
+ seq->from += n;
+ *copied = n;
+ return 0;
+}
+
+/* do_seq_show, shows the given object 'p'. If 'p' is skipped or
+ * error happens, resets seq->count to 'offs'.
+ *
+ * Returns err > 0, indicating show() skips this object.
+ * Returns err = 0, indicating show() succeeds.
+ * Returns err < 0, indicating show() fails or overflow happened.
+ */
+static int do_seq_show(struct seq_file *seq, void *p, size_t offs)
+{
+ int err;
+
+ WARN_ON(IS_ERR_OR_NULL(p));
+
+ err = seq->op->show(seq, p);
+ if (err > 0) {
+ /* object is skipped, decrease seq_num, so next
+ * valid object can reuse the same seq_num.
+ */
+ bpf_iter_dec_seq_num(seq);
+ seq->count = offs;
+ return err;
+ }
+
+ if (err < 0 || seq_has_overflowed(seq)) {
+ seq->count = offs;
+ return err ? err : -E2BIG;
+ }
+
+ /* err == 0 and no overflow */
+ return 0;
+}
+
+/* do_seq_stop, stops at the given object 'p'. 'p' could be an ERR or NULL. If
+ * 'p' is an ERR or there was an overflow, reset seq->count to 'offs' and
+ * returns error. Returns 0 otherwise.
+ */
+static int do_seq_stop(struct seq_file *seq, void *p, size_t offs)
+{
+ if (IS_ERR(p)) {
+ seq->op->stop(seq, NULL);
+ seq->count = offs;
+ return PTR_ERR(p);
+ }
+
+ seq->op->stop(seq, p);
+ if (!p) {
+ if (!seq_has_overflowed(seq)) {
+ bpf_iter_done_stop(seq);
+ } else {
+ seq->count = offs;
+ if (offs == 0)
+ return -E2BIG;
+ }
+ }
+ return 0;
+}
+
/* maximum visited objects before bailing out */
#define MAX_ITER_OBJECTS 1000000
@@ -91,7 +168,7 @@ static ssize_t bpf_seq_read(struct file *file, char __user *buf, size_t size,
loff_t *ppos)
{
struct seq_file *seq = file->private_data;
- size_t n, offs, copied = 0;
+ size_t offs, copied = 0;
int err = 0, num_objs = 0;
bool can_resched;
void *p;
@@ -108,40 +185,18 @@ static ssize_t bpf_seq_read(struct file *file, char __user *buf, size_t size,
}
if (seq->count) {
- n = min(seq->count, size);
- err = copy_to_user(buf, seq->buf + seq->from, n);
- if (err) {
- err = -EFAULT;
- goto done;
- }
- seq->count -= n;
- seq->from += n;
- copied = n;
+ err = do_copy_to_user(seq, buf, size, &copied);
goto done;
}
seq->from = 0;
p = seq->op->start(seq, &seq->index);
- if (!p)
+ if (IS_ERR_OR_NULL(p))
goto stop;
- if (IS_ERR(p)) {
- err = PTR_ERR(p);
- seq->op->stop(seq, p);
- seq->count = 0;
- goto done;
- }
- err = seq->op->show(seq, p);
- if (err > 0) {
- /* object is skipped, decrease seq_num, so next
- * valid object can reuse the same seq_num.
- */
- bpf_iter_dec_seq_num(seq);
- seq->count = 0;
- } else if (err < 0 || seq_has_overflowed(seq)) {
- if (!err)
- err = -E2BIG;
- seq->op->stop(seq, p);
+ err = do_seq_show(seq, p, 0);
+ if (err < 0) {
+ do_seq_stop(seq, p, 0);
seq->count = 0;
goto done;
}
@@ -153,7 +208,7 @@ static ssize_t bpf_seq_read(struct file *file, char __user *buf, size_t size,
num_objs++;
offs = seq->count;
p = seq->op->next(seq, p, &seq->index);
- if (pos == seq->index) {
+ if (unlikely(pos == seq->index)) {
pr_info_ratelimited("buggy seq_file .next function %ps "
"did not updated position index\n",
seq->op->next);
@@ -161,7 +216,7 @@ static ssize_t bpf_seq_read(struct file *file, char __user *buf, size_t size,
}
if (IS_ERR_OR_NULL(p))
- break;
+ goto stop;
/* got a valid next object, increase seq_num */
bpf_iter_inc_seq_num(seq);
@@ -172,22 +227,16 @@ static ssize_t bpf_seq_read(struct file *file, char __user *buf, size_t size,
if (num_objs >= MAX_ITER_OBJECTS) {
if (offs == 0) {
err = -EAGAIN;
- seq->op->stop(seq, p);
+ do_seq_stop(seq, p, seq->count);
goto done;
}
break;
}
- err = seq->op->show(seq, p);
- if (err > 0) {
- bpf_iter_dec_seq_num(seq);
- seq->count = offs;
- } else if (err < 0 || seq_has_overflowed(seq)) {
- seq->count = offs;
+ err = do_seq_show(seq, p, offs);
+ if (err < 0) {
if (offs == 0) {
- if (!err)
- err = -E2BIG;
- seq->op->stop(seq, p);
+ do_seq_stop(seq, p, seq->count);
goto done;
}
break;
@@ -197,30 +246,11 @@ static ssize_t bpf_seq_read(struct file *file, char __user *buf, size_t size,
cond_resched();
}
stop:
- offs = seq->count;
- /* bpf program called if !p */
- seq->op->stop(seq, p);
- if (!p) {
- if (!seq_has_overflowed(seq)) {
- bpf_iter_done_stop(seq);
- } else {
- seq->count = offs;
- if (offs == 0) {
- err = -E2BIG;
- goto done;
- }
- }
- }
-
- n = min(seq->count, size);
- err = copy_to_user(buf, seq->buf, n);
- if (err) {
- err = -EFAULT;
+ err = do_seq_stop(seq, p, seq->count);
+ if (err)
goto done;
- }
- copied = n;
- seq->count -= n;
- seq->from = n;
+
+ err = do_copy_to_user(seq, buf, size, &copied);
done:
if (!copied)
copied = err;
--
2.37.1.455.g008518b4e5-goog
Powered by blists - more mailing lists