netdev - Re: [PATCH 2/3] bnx2x: Casting page alignment

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-Id: <200903102148.15826.bjorn.helgaas@hp.com>
Date:	Tue, 10 Mar 2009 21:48:15 -0600
From:	Bjorn Helgaas <bjorn.helgaas@...com>
To:	"Eilon Greenstein" <eilong@...adcom.com>
Cc:	"David Miller" <davem@...emloft.net>, netdev@...r.kernel.org,
	"Alex Chiang" <achiang@...com>,
	"Vladislav Zolotarov" <vladz@...adcom.com>
Subject: Re: [PATCH 2/3] bnx2x: Casting page alignment

On Monday 09 March 2009 4:52:17 am Eilon Greenstein wrote:
> Subject: [PATCH 2/3] bnx2x: Casting page alignment
>
> Adding a proper cast to the argument of PAGE_ALIGN macro so that the output
> won't depend on its original type. Without this cast aligned value will be
> truncated to the size of the argument type.

I tested these patches (1 & 2) in the SLES11 RC5 kernel and verified
that they fix https://bugzilla.novell.com/show_bug.cgi?id=481074

However, I'm not 100% comfortable with this patch:

> diff --git a/drivers/net/bnx2x.h b/drivers/net/bnx2x.h
> index 15a5cf0..3cf2b92 100644
> --- a/drivers/net/bnx2x.h
> +++ b/drivers/net/bnx2x.h
> @@ -152,7 +152,7 @@ struct sw_rx_page {
>  #define PAGES_PER_SGE			(1 << PAGES_PER_SGE_SHIFT)
>  #define SGE_PAGE_SIZE			PAGE_SIZE
>  #define SGE_PAGE_SHIFT			PAGE_SHIFT
> -#define SGE_PAGE_ALIGN(addr)		PAGE_ALIGN(addr)
> +#define SGE_PAGE_ALIGN(addr)		PAGE_ALIGN((typeof(PAGE_SIZE))addr)

For one thing, we started with SGE_PAGE_SIZE,SHIFT,ALIGN being
exact duplicates of PAGE_SIZE,SHIFT,ALIGN, which makes me wonder
why we need the SGE_ variety.

But more importantly, the patch adds a cast in SGE_PAGE_ALIGN to
ensure that its argument doesn't get truncated in the process of
being rounded up.  This could happen anywhere, not just in bnx2x,
so my question is whether the cast should be done in PAGE_ALIGN
itself so we don't trip over this again somewhere else.

Bjorn
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html