UART driver problem

UART driver problem

[Ralf HECKHAUSEN]

I have a strange problem with the uart driver. The 8xx_write in uart.c always fails because copy_from_user returns 0 instead of the requested number of bytes. When I ignore the comparison with the requested number of bytes everything works ok, the correct bytes are transmitted and 8xx_write returns something >0.
The next strange thing: a write to a tty file descriptor sometimes returns a negative number, although the output succeded. perror in this case outputs an unknown error.
Does anyone know this problems?

Ralf


** Sent via the linuxppc-embedded mail list. See http://lists.linuxppc.org/

[PATCH] Re: UART driver problem

[Marcus Sundberg]

"Ralf HECKHAUSEN" <RHeckhau@frequentis.com> writes:

> I have a strange problem with the uart driver. The 8xx_write in uart.c
> always fails because copy_from_user returns 0 instead of the requested
> number of bytes. When I ignore the comparison with the requested
> number of bytes everything works ok, the correct bytes are transmitted
> and 8xx_write returns something >0.

I noted that as well the other day, together with another problem,
when starting to use sysvinit instead of a simple bash script.

The above problem is solved by this patch:

diff -u -u -r1.4 -r1.5
--- uart.c      2000/01/17 17:20:57     1.4
+++ uart.c      2000/01/24 18:02:19     1.5
@@ -976,8 +976,7 @@
                }

                if (from_user) {
-                       if (c !=
-                           copy_from_user(__va(bdp->cbd_bufaddr), buf, c)) {
+                       if (copy_from_user(__va(bdp->cbd_bufaddr), buf, c)) {
                                if (!ret)
                                        ret = -EFAULT;
                                break;

Strangely enough drivers/char/serial.c is broken as well in standard
kernels. The only difference is that there the copy_from_user() will
appear to always succeed instead of always failing...

The other problem got triggered by init doing open(); write(); close();
cycles on the serial console. Without this patch the write() output
gets aborted before finishing:

diff -u -u -r1.4 -r1.5
--- uart.c      2000/01/17 17:20:57     1.4
+++ uart.c      2000/01/24 18:02:19     1.5
@@ -1692,6 +1691,13 @@
         * be at least two characters waiting to be sent after the buffers
         * are empty.
         */
+       bdp = info->tx_cur;
+       /* We want to wait for the last buffer, not the first. */
+       if (bdp == info->tx_bd_base) {
+               bdp += (TX_NUM_FIFO-1);
+       } else {
+               bdp--;
+       }
        do {
 #ifdef SERIAL_DEBUG_RS_WAIT_UNTIL_SENT
                printk("lsr = %d (jiff=%lu)...", lsr, jiffies);
@@ -1703,7 +1709,6 @@
                        break;
                if (timeout && ((orig_jiffies + timeout) < jiffies))
                        break;
-               bdp = info->tx_cur;
        } while (bdp->cbd_sc & BD_SC_READY);
        current->state = TASK_RUNNING;
 #ifdef SERIAL_DEBUG_RS_WAIT_UNTIL_SENT


//Marcus
--
Signature under construction, please come back later.

** Sent via the linuxppc-embedded mail list. See http://lists.linuxppc.org/

Re: UART driver problem

[Pavel Roskin]

Hello, Marcus!

You have fixed uart.c before me (thanks to everybody who pointed it out
for me!), but your arguments were probably not quite convincing, and this
may be the reason why your patch has not yet been applied.

You write:

> Strangely enough drivers/char/serial.c is broken as well in standard
> kernels. The only difference is that there the copy_from_user() will
> appear to always succeed instead of always failing...

drivers/char/serial.c is not broken!

copy_from_user(to, from, len) returns len if it fails and 0 if it
succeeds. Now let's look into drivers/char/serial.c, function rs_write():

c -= copy_from_user(tmp_buf, buf, c);
if (!c) {
        if (!ret)
                ret = -EFAULT;
        break;
}

If copy_from_user succeeds then it returns 0 and c doesn't change. If
copy_from_user fails then it returns the value equal to c. Then c becomes
zero and the function terminates. If no data have been transferred so far,
the result code becomes -EFAULT

Indeed, this code is written in a confusing way, but it is not broken.
Anyway, I don't think that arch/ppc/8xx_io/uart.c should use this style.

Thus your patch is both correct and complete, i.e. it fixes only broken
things. I believe it should be applied both to the 2.2.x and 2.3.x
branches. It is already applied to the version used by my company.

Pavel Roskin


** Sent via the linuxppc-embedded mail list. See http://lists.linuxppc.org/

Re: UART driver problem

[Graham Stoney]

Hi guys,

Pavel Roskin writes:
> drivers/char/serial.c is not broken!

[ explaination of why the code looks correct deleted ]

Sorry to dredge up the past here, but I'm still confused about one thing:
why does write(2) to a tty return successfully when passed an invalid address?

Here's an example:

    #include <unistd.h>

    int main()
    {
	printf("bad write returned %d\n", write(1, 0, 1));
	perror("write");
	return 0;
    }

When run on Linux/PPC or Linux/i386, you get:
    bad write returned 1
    write: Success

I would have expected the SunOS result of:
    bad write returned -1
    write: Bad address

There was a discussion about this on the linux-kernel mailing list, under
the subject "bug in write(2) system call", but I can't see where to fix it.

Any clues?

Thanks,
Graham

** Sent via the linuxppc-embedded mail list. See http://lists.linuxppc.org/