Re: [PATCH v3 4/4] x86, mpx: extend siginfo structure to includebound violation information

From: Ren Qiaowei
Date: Sun Jan 26 2014 - 20:40:32 EST


On 01/27/2014 05:38 AM, David Rientjes wrote:
On Sun, 26 Jan 2014, Ren Qiaowei wrote:

arch/x86/kernel/mpx.c: In function âdo_mpx_boundsâ:
arch/x86/kernel/mpx.c:407:3: warning: cast to pointer from integer of
different size [-Wint-to-pointer-cast]
arch/x86/kernel/mpx.c:409:3: warning: cast to pointer from integer of
different size [-Wint-to-pointer-cast]

and the documentation says you explicitly want to support this config.

These types of warnings are usually indicative of real problems when
you're storing upper and lower bits in 32-bit fields after casting them
from 64-bit values.

I'm also not sure if the added fields to the generic struct siginfo can be
justified for this.

According to MPX spec, for 32-bit case, the upper 32-bits of 64-bits bound
register are ignored, and so casting to pointer from 64-bit values should be
not produce any problems.


Ok, so this is intended per the spec which nobody reading the code is
going to know and people who report the compile warnings are going to
continue to question it.

How are you planning on suppressing the warnings? It will probably
require either

- separate 64-bit and 32-bit helper functions to do_mpx_bounds() to
do appropriate casts before casting to a pointer, or

- a macro defined as a no-op for 64-bit and as a cast to 32-bit value
for 32-bit configs that will be used in do_mpx_bounds() and casted
to the pointer.

I agree with you and we should suppress all the warnings as possible. If I use (unsgined long) to cast like the following code, what do you think about it? sizeof(long) will be 4 for 32-bit.

info->si_lower = (void __user *)(unsigned long)
(xsave_buf->bndregs.bndregs[2*bndregno]);
info->si_upper = (void __user *)(unsigned long)
(~xsave_buf->bndregs.bndregs[2*bndregno+1]);

Thanks,
Qiaowei
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/