Re: [PATCH v2 0/8] drivers/usb: refactor min/max with min_t/max_t

[Andy Shevchenko <andy.shevchenko@gmail.com>]

Tue, Nov 12, 2024 at 08:58:09PM +0500, Sabyrzhan Tasbolatov kirjoitti:
> This patch series improves type safety in the drivers/usb/*
> by using `min_t()` and `max_t()` instead of min(), max()
> with the casting inside. It should address potential type promotion issues.
> 
> Scanned the current drivers/usb code with `max\(.*\(` and `min\(.*\(`
> regexp queries to find casting inside of min() and max() which
> may lead to subtle bugs or even security vulnerabilities,
> especially if negative values are involved.
> 
> Let's refactor to min_t() and max_t() specifying the data type
> to ensure it's applicable for the both compareable arguments.

max_t() is okay to use, but min_t() is quite a beast. Please, reconsider the
entire series and tell how had this been tested? I can't imagine how many
subtle bugs it might have introduced.

min_t() explicitly casts to the given type and this is a huge problem for
the cases when one of the parameter is of signed type. Besisdes that it
diminishes the type checking done in the min().

As Linus told, the many cases when you want to have min_t() is actually 
clamp(). In some cases you wanted umin().

+Cc: David.


-- 
With Best Regards,
Andy Shevchenko