Linux-EDAC Archive on lore.kernel.org
 help / color / Atom feed
From: Borislav Petkov <bp@alien8.de>
To: Robert Richter <rrichter@marvell.com>
Cc: Mauro Carvalho Chehab <mchehab@kernel.org>,
	Tony Luck <tony.luck@intel.com>,
	James Morse <james.morse@arm.com>,
	Aristeu Rozanski <aris@redhat.com>,
	Matthias Brugger <mbrugger@suse.com>,
	linux-edac@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH v2 02/10] EDAC/mc: Use int type for parameters of edac_mc_alloc()
Date: Thu, 23 Apr 2020 19:49:34 +0200
Message-ID: <20200423174934.GF26021@zn.tnic> (raw)
In-Reply-To: <20200422115814.22205-3-rrichter@marvell.com>

On Wed, Apr 22, 2020 at 01:58:06PM +0200, Robert Richter wrote:
> Most iterators use int type as index. mci->mc_idx is also type int. So
> use int type for parameters of edac_mc_alloc(). Extend the range check
> to check for negative values. There is a type cast now when assigning
> variable n_layers to mci->n_layer, it is safe due to the range check.
> 
> While at it, rename the users of edac_mc_alloc() to mc_idx as this
> fits better here.
> 
> Signed-off-by: Robert Richter <rrichter@marvell.com>
> ---
>  drivers/edac/edac_mc.c | 7 +++----
>  drivers/edac/edac_mc.h | 6 +++---
>  2 files changed, 6 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/edac/edac_mc.c b/drivers/edac/edac_mc.c
> index 107d7c4de933..57d1d356d69c 100644
> --- a/drivers/edac/edac_mc.c
> +++ b/drivers/edac/edac_mc.c
> @@ -444,8 +444,7 @@ static int edac_mc_alloc_dimms(struct mem_ctl_info *mci)
>  	return 0;
>  }
>  
> -struct mem_ctl_info *edac_mc_alloc(unsigned int mc_num,
> -				   unsigned int n_layers,
> +struct mem_ctl_info *edac_mc_alloc(int mc_idx, int n_layers,
>  				   struct edac_mc_layer *layers,
>  				   unsigned int sz_pvt)
>  {
> @@ -456,7 +455,7 @@ struct mem_ctl_info *edac_mc_alloc(unsigned int mc_num,
>  	void *pvt, *ptr = NULL;
>  	bool per_rank = false;
>  
> -	if (WARN_ON(n_layers > EDAC_MAX_LAYERS || n_layers == 0))
> +	if (WARN_ON(mc_idx < 0 || n_layers < 1 || n_layers > EDAC_MAX_LAYERS))
>  		return NULL;

Yeah, no, this doesn't make sense to me. The memory controller number
and the number of layers can never ever be negative and thus signed.

And some drivers supply unsigned types and some signed. So if anything,
this should be fixing all the callers to supply unsigned quantities.

Thx.

-- 
Regards/Gruss,
    Boris.

https://people.kernel.org/tglx/notes-about-netiquette

  reply index

Thread overview: 31+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-04-22 11:58 [PATCH v2 00/10] EDAC/mc/ghes: Fixes, cleanup and reworks Robert Richter
2020-04-22 11:58 ` [PATCH v2 01/10] EDAC/mc: Fix usage of snprintf() and dimm location setup Robert Richter
2020-04-22 20:52   ` Borislav Petkov
2020-05-19  9:27     ` Robert Richter
2020-04-22 11:58 ` [PATCH v2 02/10] EDAC/mc: Use int type for parameters of edac_mc_alloc() Robert Richter
2020-04-23 17:49   ` Borislav Petkov [this message]
2020-05-19  9:33     ` Robert Richter
2020-04-22 11:58 ` [PATCH v2 03/10] EDAC/ghes: Remove unused members of struct ghes_edac_pvt, rename it to ghes_mci Robert Richter
2020-04-23 17:55   ` Borislav Petkov
2020-05-05  7:50     ` Robert Richter
2020-04-22 11:58 ` [PATCH v2 04/10] EDAC/ghes: Make SMBIOS handle private data to ghes Robert Richter
2020-04-24 12:12   ` kbuild test robot
2020-04-24 16:21   ` Borislav Petkov
2020-05-05 12:48     ` Robert Richter
2020-04-22 11:58 ` [PATCH v2 05/10] EDAC/ghes: Setup DIMM label from DMI and use it in error reports Robert Richter
2020-04-22 11:58 ` [PATCH v2 06/10] EDAC/ghes: Remove local variable rdr_mask in ghes_edac_dmidecode() Robert Richter
2020-04-27  7:08   ` Borislav Petkov
2020-04-27 17:24     ` Luck, Tony
2020-04-27 17:34       ` Borislav Petkov
2020-05-19  9:34         ` Robert Richter
2020-04-22 11:58 ` [PATCH v2 07/10] EDAC/ghes: Cleanup struct ghes_edac_dimm_fill, rename it to ghes_dimm_fill Robert Richter
2020-04-27 14:00   ` Borislav Petkov
2020-05-19  9:35     ` Robert Richter
2020-04-22 11:58 ` [PATCH v2 08/10] EDAC/ghes: Carve out MC device handling into separate functions Robert Richter
2020-04-27 16:38   ` Borislav Petkov
2020-05-06  8:45     ` Robert Richter
2020-05-11 13:32       ` Borislav Petkov
2020-05-19  9:57         ` Robert Richter
2020-04-22 11:58 ` [PATCH v2 09/10] EDAC/ghes: Have a separate code path for creating the fake MC Robert Richter
2020-04-22 11:58 ` [PATCH v2 10/10] EDAC/ghes: Carve out code into ghes_edac_register_{one,fake}() Robert Richter
2020-05-06  8:53 ` [PATCH v2 00/10] EDAC/mc/ghes: Fixes, cleanup and reworks Robert Richter

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20200423174934.GF26021@zn.tnic \
    --to=bp@alien8.de \
    --cc=aris@redhat.com \
    --cc=james.morse@arm.com \
    --cc=linux-edac@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mbrugger@suse.com \
    --cc=mchehab@kernel.org \
    --cc=rrichter@marvell.com \
    --cc=tony.luck@intel.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

Linux-EDAC Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/linux-edac/0 linux-edac/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 linux-edac linux-edac/ https://lore.kernel.org/linux-edac \
		linux-edac@vger.kernel.org
	public-inbox-index linux-edac

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.linux-edac


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git