On Mon, Aug 01, 2022 at 01:50:39PM -0700, Hao Luo wrote:
SNIP
> - 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;
we could still keep the break here
>
> /* 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;
looks like we tried to copy the data before when stop failed,
now it's skipped
jirka
> - }
> - 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
>
