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  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Date:   Thu, 16 May 2019 18:20:58 +0100
From:   Alan Cox <gnomes@...rguk.ukuu.org.uk>
To:     Arnaud Pouliquen <arnaud.pouliquen@...com>
Cc:     Ohad Ben-Cohen <ohad@...ery.com>,
        Bjorn Andersson <bjorn.andersson@...aro.org>,
        Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
        Jiri Slaby <jslaby@...e.com>,
        xiang xiao <xiaoxiang781216@...il.com>,
        <linux-kernel@...r.kernel.org>, <linux-remoteproc@...r.kernel.org>,
        Fabien DESSENNE <fabien.dessenne@...com>
Subject: Re: [PATCH v2 2/2] tty: add rpmsg driver


> +static int rpmsg_tty_data_handler(struct rpmsg_device *rpdev, void *data,
> +				  int len, void *priv, u32 src)
> +{
> +	struct rpmsg_tty_port *cport = dev_get_drvdata(&rpdev->dev);
> +	u8 *cbuf;
> +	int space;
> +
> +	dev_dbg(&rpdev->dev, "msg(<- src 0x%x) len %d\n", src, len);
> +
> +	if (!len)
> +		return 0;
> +
> +	dev_dbg(&rpdev->dev, "space available: %d\n",
> +		tty_buffer_space_avail(&cport->port));
> +
> +	space = tty_prepare_flip_string(&cport->port, &cbuf, len);
> +	if (space <= 0) {
> +		dev_err(&rpdev->dev, "No memory for tty_prepare_flip_string\n");
> +		return -ENOMEM;
> +	}

Really bad idea.

1. It's not an 'error'. It's normal that in the case the consumer is
blocked you can run out of processing space

2. You will trigger this when being hit by a very large fast flow of data
so responding by burning CPU spewing messages (possibly even out of this
tty) is bad.

It's not a bug - just keep statistics and throw it away. If anyone cares
they will do flow control.


> +
> +static int rpmsg_tty_write_control(struct tty_struct *tty, u8 ctrl, u8 *values,
> +				   unsigned int n_value)
> +{
> +	struct rpmsg_tty_port *cport = idr_find(&tty_idr, tty->index);
> +	struct rpmsg_tty_payload *msg;
> +	struct rpmsg_tty_ctrl *m_ctrl;
> +	struct rpmsg_device *rpdev;
> +	unsigned int msg_size;
> +	int ret;
> +
> +	if (!cport) {
> +		dev_err(tty->dev, "cannot get cport\n");
> +		return -ENODEV;
> +	}
> +
> +	rpdev = cport->rpdev;
> +
> +	msg_size = sizeof(*msg) + sizeof(*m_ctrl) + n_value;
> +	msg = kzalloc(msg_size, GFP_KERNEL);


Out of memory check ?

> +static int rpmsg_tty_write(struct tty_struct *tty, const u8 *buf, int len)
> +{
> +	struct rpmsg_tty_port *cport = idr_find(&tty_idr, tty->index);
> +	struct rpmsg_device *rpdev;
> +	int msg_size, msg_max_size, ret = 0;
> +	int cmd_sz = sizeof(struct rpmsg_tty_payload);
> +	u8 *tmpbuf;
> +
> +	if (!cport) {
> +		dev_err(tty->dev, "cannot get cport\n");
> +		return -ENODEV;
> +	}
> +
> +	/* If cts not set, the message is not sent*/
> +	if (!cport->cts)
> +		return 0;
> +
> +	rpdev = cport->rpdev;
> +
> +	dev_dbg(&rpdev->dev, "%s: send msg from tty->index = %d, len = %d\n",
> +		__func__, tty->index, len);
> +	if (!buf) {
> +		dev_err(&rpdev->dev, "buf shouldn't be null.\n");
> +		return -ENOMEM;
> +	}
> +
> +	msg_max_size = rpmsg_get_buf_payload_size(rpdev->ept);
> +	if (msg_max_size < 0)
> +		return msg_max_size;
> +
> +	msg_size = min(len + cmd_sz, msg_max_size);
> +	tmpbuf = kzalloc(msg_size, GFP_KERNEL);

Allocation failure check ?

Powered by blists - more mailing lists