If a user does not have CAP_SYS_ADMIN and runs this code: if (ioctl(fd, TIOCGSERIAL, &serialUart) == -1) { printf("ioctl(TIOCGSERIAL) failed: %s\n", strerror(errno)); close(fd); fd = -1; return fd; } // Now set the serial configuration with low latency set. serialUart.flags |= ASYNC_LOW_LATENCY; if (ioctl(fd, TIOCSSERIAL, &serialUart) == -1) { printf("ioctl(TIOCSSERIAL) failed: %s\n", strerror(errno)); close(fd); fd = -1; return fd; } Code inside uart_set_info will temporarily set retval to -EPERM and execute to "goto check_and_exit" if (!capable(CAP_SYS_ADMIN)) { retval = -EPERM; if (change_irq || change_port || (new_info->baud_base != uport->uartclk / 16) || (close_delay != port->close_delay) || (closing_wait != port->closing_wait) || (new_info->xmit_fifo_size && new_info->xmit_fifo_size != uport->fifosize) || (((new_flags ^ old_flags) & ~UPF_USR_MASK) != 0)) goto exit; uport->flags = ((uport->flags & ~UPF_USR_MASK) | (new_flags & UPF_USR_MASK)); uport->custom_divisor = new_info->custom_divisor; goto check_and_exit; } 1. Should uport->flags and uport->custom_divisor still get set? Additionally, first line of code for check_and_exit clears ret_val = -EPERM by setting it back to 0: check_and_exit: retval = 0; if (uport->type == PORT_UNKNOWN) goto exit; if (tty_port_initialized(port)) { if (((old_flags ^ uport->flags) & UPF_SPD_MASK) || old_custom_divisor != uport->custom_divisor) { /* * If they're setting up a custom divisor or speed, * instead of clearing it, then bitch about it. */ if (uport->flags & UPF_SPD_MASK) { dev_notice_ratelimited(uport->dev, "%s sets custom speed on %s. This is deprecated.\n", current->comm, tty_name(port->tty)); } uart_change_speed(tty, state, NULL); } } else { retval = uart_startup(tty, state, 1); if (retval == 0) tty_port_set_initialized(port, true); if (retval > 0) retval = 0; } exit: return retval; Therefore the -EPERM ret_val is lost and the user is unaware of the capabilities rejection.
(In reply to nlowell from comment #0) > if (!capable(CAP_SYS_ADMIN)) { > retval = -EPERM; > if (change_irq || change_port || > (new_info->baud_base != uport->uartclk / 16) || > (close_delay != port->close_delay) || > (closing_wait != port->closing_wait) || > (new_info->xmit_fifo_size && > new_info->xmit_fifo_size != uport->fifosize) || > (((new_flags ^ old_flags) & ~UPF_USR_MASK) != 0)) > goto exit; If a user set something inappropriate, we go to "exit" -- return -EPERM. > uport->flags = ((uport->flags & ~UPF_USR_MASK) | > (new_flags & UPF_USR_MASK)); > uport->custom_divisor = new_info->custom_divisor; > goto check_and_exit; If they did an allowed thing, we go to check_and_exit -- return 0. > } > > 1. Should uport->flags and uport->custom_divisor still get set? Yes, a user can set UPF_USR_MASK flags to uport->flags. > Additionally, first line of code for check_and_exit clears ret_val = -EPERM > by setting it back to 0: > > check_and_exit: > retval = 0; Which is exactly what is intended. > Therefore the -EPERM ret_val is lost and the user is unaware of the > capabilities rejection. Rejection of what? Users are supposed to check via TIOCGSERIAL again.
Thank you for the explanation. I see now that port->low_latency = (uport->flags & UPF_LOW_LATENCY) ? 1 : 0; which used to be just before check_and_exit label has been removed from newer kernel versions (I have been working with v5.4). So the issue was uport->flags was getting set but port->low_latency was not, which I believe is a bug, but perhaps that was the only item getting skipped that mattered. So perhaps this is a false alarm for the latest kernel. My apologies. Side note: It's confusing that ret gets set to -EPERM before the additional checking which could end up negating that change. Perhaps it would be better to only set ret = -EPERM before "goto exit".