Messages in this thread | | | Subject | Re: [PATCH 2/6] tty: convert tty_ldisc_ops 'read()' function to take a kernel pointer | From | Jiri Slaby <> | Date | Thu, 21 Jan 2021 12:02:03 +0100 |
| |
On 21. 01. 21, 10:00, Greg Kroah-Hartman wrote: > From: Linus Torvalds <torvalds@linux-foundation.org> > > The tty line discipline .read() function was passed the final user > pointer destination as an argument, which doesn't match the 'write()' > function, and makes it very inconvenient to do a splice method for > ttys. > > This is a conversion to use a kernel buffer instead. > > NOTE! It does this by passing the tty line discipline ->read() function > an additional "cookie" to fill in, and an offset into the cookie data. > > The line discipline can fill in the cookie data with its own private > information, and then the reader will repeat the read until either the > cookie is cleared or it runs out of data. > > The only real user of this is N_HDLC, which can use this to handle big > packets, even if the kernel buffer is smaller than the whole packet. > > Cc: Christoph Hellwig <hch@lst.de> > Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org> > Cc: Al Viro <viro@zeniv.linux.org.uk> > Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org> ... > --- a/drivers/tty/tty_io.c > +++ b/drivers/tty/tty_io.c > @@ -833,6 +833,65 @@ static void tty_update_time(struct timespec64 *time) > time->tv_sec = sec; > } > > +/* > + * Iterate on the ldisc ->read() function until we've gotten all > + * the data the ldisc has for us. > + * > + * The "cookie" is something that the ldisc read function can fill > + * in to let us know that there is more data to be had. > + * > + * We promise to continue to call the ldisc until it stops returning > + * data or clears the cookie. The cookie may be something that the > + * ldisc maintains state for and needs to free. > + */ > +static int iterate_tty_read(struct tty_ldisc *ld, struct tty_struct *tty, struct file *file, > + char __user *buf, size_t count) > +{ > + int retval = 0; > + void *cookie = NULL; > + unsigned long offset = 0; > + char kernel_buf[64]; > + > + do { > + int size, uncopied; > + > + size = count > sizeof(kernel_buf) ? sizeof(kernel_buf) : count;
Or simply size = min(count, sizeof(kernel_buf));
> + size = ld->ops->read(tty, file, kernel_buf, size, &cookie, offset); > + if (!size) > + break; > + > + /* > + * A ldisc read error return will override any previously copied > + * data (eg -EOVERFLOW from HDLC) > + */ > + if (size < 0) { > + memzero_explicit(kernel_buf, sizeof(kernel_buf)); > + return size; > + } > + > + uncopied = copy_to_user(buf+offset, kernel_buf, size); > + size -= uncopied; > + offset += size; > + count -= size; > + > + /* > + * If the user copy failed, we still need to do another ->read() > + * call if we had a cookie to let the ldisc clear up. > + * > + * But make sure size is zeroed. > + */ > + if (unlikely(uncopied)) { > + count = 0; > + retval = -EFAULT;
n_hdlc_tty_read will return EOVERFLOW when size is 0, so this EFAULT is never propagated, if I am looking correctly? n_tty seems to be fine (returns zero for zeroed size).
> + } > + } while (cookie); > + > + /* We always clear tty buffer in case they contained passwords */ > + memzero_explicit(kernel_buf, sizeof(kernel_buf)); > + return offset ? offset : retval; > +}
thanks, -- js
| |