Messages in this thread | | | From | Pietro Borrello <> | Date | Sat, 28 Jan 2023 17:57:06 +0100 | Subject | Re: [PATCH net-next] net/tls: tls_is_tx_ready() checked list_entry |
| |
On Sat, 28 Jan 2023 at 17:40, Simon Horman <simon.horman@corigine.com> wrote: > > Hi Pietro, > > I agree this is correct. > > However, given that the code has been around for a while, > I feel it's relevant to ask if tx_list can ever be NULL. > If not, perhaps it's better to remove the error path entirely.
Hi Simon, Thank you for your fast reply. The point is exactly that tx_list will never be NULL, as the list head will always be present. So the error, as is, will never be detected, resulting in type confusion. We found this with static analysis, so we have no way to say for sure that the list can never be empty on edge cases. As this is a type confusion, the errors are often sneaky and go undetected.
As an example, the following bug we previously reported resulted in a type confusion on net code that went undetected for more than 20 years. Link: https://lore.kernel.org/all/9fcd182f1099f86c6661f3717f63712ddd1c676c.1674496737.git.marcelo.leitner@gmail.com/ In that case, we were able to create a PoC to demonstrate the issue where we leveraged the type confusion to bypass KASLR.
In the end, this is the maintainer's call, but I would keep the check and correctly issue a list_first_entry_or_null() so that the check will work as intended as the added overhead is just a pointer comparison which would likely justify the cost of a more solid code. Otherwise, I can also submit a patch that entirely removes the check. Let me know what you prefer.
Best regards, Pietro
| |