Posted yesterday to linux-serial with no response.
The OX16PCI954 UART contains a 9bit mode. I'm developing support for
it. I thought it would be easy to shoehorn into the UPF_* flags:
diff -Naurp linux-2.6.11-5-titan485/include/linux/serial_core.h linux-2.6.11-6-9bit/include/linux/serial_core.h
--- linux-2.6.11-5-titan485/include/linux/serial_core.h 2006-06-02 13:59:07.000000000 -0400
+++ linux-2.6.11-6-9bit/include/linux/serial_core.h 2006-06-07 18:11:51.000000000 -0400
@@ -219,6 +219,7 @@ struct uart_port {
#define UPF_SKIP_TEST (1 << 6)
#define UPF_AUTO_IRQ (1 << 7)
#define UPF_HARDPPS_CD (1 << 11)
+#define UPF_9BIT (1 << 12)
#define UPF_LOW_LATENCY (1 << 13)
#define UPF_BUGGY_UART (1 << 14)
#define UPF_AUTOPROBE (1 << 15)
However, in serial_core.c:set_uart_info(), there is a problem. The
flag should be within the purview of UPF_USR_MASK so that
non-privileged users can turn it on or off, and yet, I don't want the
mode to be enabled on UARTs that don't have it which requires
verification from the low-level driver. There is only one call to
ops->verify_port(), and it's not in the correct place for this to
happen.
So, I initially thought this patch would be best:
diff -Naurp linux-2.6.11-5-titan485/drivers/serial/serial_core.c linux-2.6.11-6-9bit/drivers/serial/serial_core.c
--- linux-2.6.11-5-titan485/drivers/serial/serial_core.c 2006-06-07 16:01:44.000000000 -0400
+++ linux-2.6.11-6-9bit/drivers/serial/serial_core.c 2006-06-08 11:08:00.000000000 -0400
@@ -647,6 +647,12 @@ static int uart_set_info(struct uart_sta
old_flags = port->flags;
old_custom_divisor = port->custom_divisor;
+ /*
+ * Ask the low level driver to verify the settings.
+ */
+ if (port->ops->verify_port)
+ retval = port->ops->verify_port(port, &new_serial);
+
if (!capable(CAP_SYS_ADMIN)) {
retval = -EPERM;
if (change_irq || change_port ||
@@ -662,12 +668,6 @@ static int uart_set_info(struct uart_sta
goto check_and_exit;
}
- /*
- * Ask the low level driver to verify the settings.
- */
- if (port->ops->verify_port)
- retval = port->ops->verify_port(port, &new_serial);
-
if ((new_serial.irq >= NR_IRQS) || (new_serial.irq < 0) ||
(new_serial.baud_base < 9600))
retval = -EINVAL;
but I'm not sure that's not a security hole of some sort; revealing
that the setting is valid or invalid before revealing whether the user
is allowed to set it. So perhaps this is better:
diff -Naurp linux-2.6.11-5-titan485/drivers/serial/serial_core.c linux-2.6.11-6-9bit/drivers/serial/serial_core.c
--- linux-2.6.11-5-titan485/drivers/serial/serial_core.c 2006-06-07 16:01:44.000000000 -0400
+++ linux-2.6.11-6-9bit/drivers/serial/serial_core.c 2006-06-08 11:45:16.000000000 -0400
@@ -656,6 +656,14 @@ static int uart_set_info(struct uart_sta
(new_serial.xmit_fifo_size != port->fifosize) ||
(((new_serial.flags ^ old_flags) & ~UPF_USR_MASK) != 0))
goto exit;
+ /*
+ * Ask the low level driver to verify the settings.
+ */
+ if (port->ops->verify_port) {
+ retval = port->ops->verify_port(port, &new_serial);
+ if (retval)
+ goto exit;
+ }
port->flags = ((port->flags & ~UPF_USR_MASK) |
(new_serial.flags & UPF_USR_MASK));
port->custom_divisor = new_serial.custom_divisor;
but I don't like the duplication of code.
Any thoughts?
..Stu
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[Index of Archives]
[Kernel Newbies]
[Netfilter]
[Bugtraq]
[Photo]
[Stuff]
[Gimp]
[Yosemite News]
[MIPS Linux]
[ARM Linux]
[Linux Security]
[Linux RAID]
[Video 4 Linux]
[Linux for the blind]
[Linux Resources]
