Re: [PATCH] Bluetooth: L2CAP: don't check for out-of-bounds value

[Dan Carpenter <dan.carpenter@xxxxxxxxxx>]

On Fri, May 26, 2023 at 04:16:54PM +0200, Simon Horman wrote:
> @@ -4161,8 +4161,12 @@ static struct l2cap_chan *l2cap_connect(struct l2cap_conn *conn,
>  
>  	result = L2CAP_CR_NO_MEM;
>  
> -	/* Check for valid dynamic CID range (as per Erratum 3253) */
> -	if (scid < L2CAP_CID_DYN_START || scid > L2CAP_CID_DYN_END) {
> +	/* Check for valid dynamic CID range (as per Erratum 3253).
> +	 * As scid is an unsigned 16bit variable it's maximum
> +	 * value is L2CAP_CID_DYN_END (0xffff): there is no need to check
> +	 * if scid exceeds that value here.
> +	 */
> +	if (scid < L2CAP_CID_DYN_START) {

This is a false positive.  To me the warning looks reasonable.  But one
way we could silence it would be to keep a list of macros where the
check is impossible but we still want to have it.

I could create something where we do:

echo "L2CAP_CID_DYN_END" >> smatch_data/kernel.allowed_impossible_limits

I'd do the same for unsigned comparisons with zero like:


        if (dpmcp_dev->obj_desc.ver_major < DPMCP_MIN_VER_MAJOR ||
            (dpmcp_dev->obj_desc.ver_major == DPMCP_MIN_VER_MAJOR &&
             dpmcp_dev->obj_desc.ver_minor < DPMCP_MIN_VER_MINOR)) {
             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
                dev_err(&dpmcp_dev->dev,
                        "ERROR: Version %d.%d of DPMCP not supported.\n",

echo "DPMCP_MIN_VER_MINOR" >> smatch_data/kernel.allowed_impossible_limits

I can do that on Monday if you want.  Other static checkers might
complain still though.

regards,
dan carpenter