Re: [PATCH] ASoC: qcom: q6dsp: Fix an off-by-one in q6adm_alloc_copp()

From: Dan Carpenter <dan.carpenter@oracle.com>
To: Christophe JAILLET <christophe.jaillet@wanadoo.fr>
Cc: alsa-devel@alsa-project.org,
	Banajit Goswami <bgoswami@codeaurora.org>,
	Harshit Mogalapalli <harshit.m.mogalapalli@oracle.com>,
	linux-kernel@vger.kernel.org, kernel-janitors@vger.kernel.org,
	Takashi Iwai <tiwai@suse.com>,
	Liam Girdwood <lgirdwood@gmail.com>,
	Mark Brown <broonie@kernel.org>,
	Srinivas Kandagatla <srinivas.kandagatla@linaro.org>,
	Banajit Goswami <bgoswami@quicinc.com>
Subject: Re: [PATCH] ASoC: qcom: q6dsp: Fix an off-by-one in q6adm_alloc_copp()
Date: Thu, 21 Jul 2022 13:47:31 +0300	[thread overview]
Message-ID: <20220721104731.GK2316@kadam> (raw)
In-Reply-To: <a29ea047-4935-4893-108b-f29f46971272@wanadoo.fr>

[-- Attachment #1: Type: text/plain, Size: 844 bytes --]

On Thu, Jul 21, 2022 at 12:30:32PM +0200, Christophe JAILLET wrote:
> You could add find_last_bit(), find_next_zero_bit_le() and
> find_next_bit_le().
> 

Thanks!

> > 
> > regards,
> > dan carpenter
> > 
> > 
> 
> A reduced version of mine was:
> 
> @@
> expression e1, e2;
> statement S;
> @@
> (
> *   e1 = find_first_bit(...);
> |
> *   e1 = find_last_bit(...);
> |
> 	[... snip ...]
> )
>     ...
>     if (e1 > e2)
>         S
> 
> 
> (and it takes only a few seconds to scan the whole kernel :) )

Nice!

I wasn't going to be before but now I have to re-write my generic
check to be even more *powerful* than before!  The new check doesn't
rely on known values for the limit, but uses comparison data instead.

(Still takes overnight to run so I might end up sorely dissappointed
and defeated tomorrow morning)

regards,
dan carpenter

[-- Attachment #2: check_off_by_one_capped_return.c --]
[-- Type: text/x-csrc, Size: 1528 bytes --]

/*
 * Copyright (C) 2022 Oracle.
 *
 * This program is free software; you can redistribute it and/or
 * modify it under the terms of the GNU General Public License
 * as published by the Free Software Foundation; either version 2
 * of the License, or (at your option) any later version.
 *
 * This program is distributed in the hope that it will be useful,
 * but WITHOUT ANY WARRANTY; without even the implied warranty of
 * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
 * GNU General Public License for more details.
 *
 * You should have received a copy of the GNU General Public License
 * along with this program; if not, see http://www.gnu.org/copyleft/gpl.txt
 */

#include "smatch.h"
#include "smatch_extra.h"

static int my_id;

static void match_condition(struct expression *expr)
{
	struct expression *prev;
	int comparison;
	sval_t sval;
	char *name;

	if (expr->type != EXPR_COMPARE)
		return;
	if (expr->op != '>' && expr->op != SPECIAL_UNSIGNED_GT)
		return;

	if (!get_implied_value(expr, &sval) || sval.value != 0)
		return;

	comparison = get_comparison(expr->left, expr->right);
	if (!comparison)
		return;
	if (show_special(comparison)[1] != '=')
		return;

	prev = get_assigned_expr(expr->left);
	prev = strip_expr(prev);
	if (!prev || prev->type != EXPR_CALL)
		return;

	name = expr_to_str(prev->fn);
	sm_warning("potential off by one check '%s()'", name);
	free_string(name);
}

void check_off_by_one_capped_return(int id)
{
	my_id = id;

	add_hook(&match_condition, CONDITION_HOOK);
}