Bug 215205
Summary: | uart_set_info not returning -EPERM when incapable user tries to set ASYNC_LOW_LATENCY | ||
---|---|---|---|
Product: | Drivers | Reporter: | nlowell |
Component: | Serial | Assignee: | Russell King (rmk) |
Status: | NEW --- | ||
Severity: | normal | CC: | jirislaby |
Priority: | P1 | ||
Hardware: | All | ||
OS: | Linux | ||
Kernel Version: | HEAD | Subsystem: | |
Regression: | No | Bisected commit-id: |
Description
nlowell
2021-12-02 20:52:10 UTC
(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". |