Re: Can this be a invalid memory access? (was: Re: [PATCH] spi/xilinx: Cast ioread32/iowrite32 function pointers)
Hi Ricardo,
On Mon, Feb 2, 2015 at 3:05 PM, Ricardo Ribalda Delgado
wrote:
> On Mon, Feb 2, 2015 at 2:04 PM, Geert Uytterhoeven
> wrote:
>>> void * pvar=varB;
>>
>> ... =
>>
>>> *pvar = ioread8(valid_memory);
>>>
>>> Depending if ioread8 returns a u8 or a unsigned int, aren't we also
>>> accessing varC?
>>>
>>> Could not this be a problem?
>>
>> Please try to compile the above.
>> The compiler will tell you you cannot dereference a void pointer.
>>
>> Now, replace "void *" by "unsigned int *".
>> After that, varC will indeed be overwritten. But the compiler will still have
>> warned you that you assigned the address of an u8 variable to an
>> unsigned int pointer.
>
> I am still a bit worried, that the same function has different
> prototypes and implementations depending on the arch. But
> "unfortunately" I cannot make a counter-example that can cause an
> error.
>
> So far, the closest I have get is this:
>
> ricardo@neopili:/tmp$ cat arch.c
> #include
You should include io.h here...
>
> unsigned int my_ioread8(){
... then the compiler will complain here.
> return 0xdeadbeef;
> }
>
> void test_read();
> int main(int argc, char *argv[]){
>
> test_read();
> return 0;
> }
>
>
> ricardo@neopili:/tmp$ cat io.h
> #ifndef IO_H
> #define IO_H
> #include
>
> uint8_t my_ioread8();
>
> #endif
>
>
> ricardo@neopili:/tmp$ cat driver.c
> #include "io.h"
> #include
> #include
>
> void test_read(){
> uint8_t varA=0x1;
> uint8_t varB=0x2;
> uint8_t varC=0x3;
>
> varB = my_ioread8();
>
> fprintf(stdout, "varA=%d varB=%d varC=%d\n",
> varA, varB, varC);
> }
>
> It does not produce any error at build time:
> ricardo@neopili:/tmp$ make
> cc -Wall -c -o arch.o arch.c
> cc -Wall -c -o driver.o driver.c
> cc -Wall arch.o driver.o -o test
>
> and it works fine on amd86
>
> ricardo@neopili:/tmp$ ./test
> varA=1 varB=239 varC=3
>
> But in the hypothetical case where the calling convention will be
> different for unsigned int and u8, there will be an issue
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: Can this be a invalid memory access? (was: Re: [PATCH] spi/xilinx: Cast ioread32/iowrite32 function pointers)
Hello Geert
On Mon, Feb 2, 2015 at 2:04 PM, Geert Uytterhoeven wrote:
>> void * pvar=varB;
>
> ... =
>
>> *pvar = ioread8(valid_memory);
>>
>> Depending if ioread8 returns a u8 or a unsigned int, aren't we also
>> accessing varC?
>>
>> Could not this be a problem?
>
> Please try to compile the above.
> The compiler will tell you you cannot dereference a void pointer.
>
> Now, replace "void *" by "unsigned int *".
> After that, varC will indeed be overwritten. But the compiler will still have
> warned you that you assigned the address of an u8 variable to an
> unsigned int pointer.
I am still a bit worried, that the same function has different
prototypes and implementations depending on the arch. But
"unfortunately" I cannot make a counter-example that can cause an
error.
So far, the closest I have get is this:
ricardo@neopili:/tmp$ cat arch.c
#include
unsigned int my_ioread8(){
return 0xdeadbeef;
}
void test_read();
int main(int argc, char *argv[]){
test_read();
return 0;
}
ricardo@neopili:/tmp$ cat io.h
#ifndef IO_H
#define IO_H
#include
uint8_t my_ioread8();
#endif
ricardo@neopili:/tmp$ cat driver.c
#include "io.h"
#include
#include
void test_read(){
uint8_t varA=0x1;
uint8_t varB=0x2;
uint8_t varC=0x3;
varB = my_ioread8();
fprintf(stdout, "varA=%d varB=%d varC=%d\n",
varA, varB, varC);
}
It does not produce any error at build time:
ricardo@neopili:/tmp$ make
cc -Wall -c -o arch.o arch.c
cc -Wall -c -o driver.o driver.c
cc -Wall arch.o driver.o -o test
and it works fine on amd86
ricardo@neopili:/tmp$ ./test
varA=1 varB=239 varC=3
But in the hypothetical case where the calling convention will be
different for unsigned int and u8, there will be an issue
The reason that I am interested in this is that I want to be able to
have code like:
struct priv_data{
u16 (read *)(void __iomem *);
};
endian=detect_endianness()
if (endian == BIG)
priv->read= ioread16be;
else
priv->read= ioread16;
data = priv->read(iomem+DATA);
Unfortunately, with the current code, this does not work on all the
arches, and the workaround is having wrappers for ioread and iowrite
like in drivers/spi/spi-xilinx.c
static void xspi_write32(u32 val, void __iomem *addr)
{
iowrite32(val, addr);
}
static void xspi_write32_be(u32 val, void __iomem *addr)
{
iowrite32be(val, addr);
}
Regards
--
Ricardo Ribalda
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: Can this be a invalid memory access? (was: Re: [PATCH] spi/xilinx: Cast ioread32/iowrite32 function pointers)
Hi Ricardo,
On Mon, Feb 2, 2015 at 1:49 PM, Ricardo Ribalda Delgado
wrote:
> Regarding ioread8 et al.
>
> On include/asm-generic/io.h is defined as:
> extern unsigned int ioread8(void __iomem *);
>
> On include/asm-generic/io.h:
> static inline u8 ioread8(const volatile void __iomem *addr)
>
> Please ignore the qualifiers right now. The first function returns an
> unsigned integer, the second a u8.
>
> Through #ifdefs, different arches uses the first or the second definitions.
>
> If we consider this code:
>
> u8 varA;
> u8 varB;
> u8 varC;
> void * pvar=varB;
... =
> *pvar = ioread8(valid_memory);
>
> Depending if ioread8 returns a u8 or a unsigned int, aren't we also
> accessing varC?
>
> Could not this be a problem?
Please try to compile the above.
The compiler will tell you you cannot dereference a void pointer.
Now, replace "void *" by "unsigned int *".
After that, varC will indeed be overwritten. But the compiler will still have
warned you that you assigned the address of an u8 variable to an
unsigned int pointer.
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Can this be a invalid memory access? (was: Re: [PATCH] spi/xilinx: Cast ioread32/iowrite32 function pointers)
Hello Regarding ioread8 et al. On include/asm-generic/io.h is defined as: extern unsigned int ioread8(void __iomem *); On include/asm-generic/io.h: static inline u8 ioread8(const volatile void __iomem *addr) Please ignore the qualifiers right now. The first function returns an unsigned integer, the second a u8. Through #ifdefs, different arches uses the first or the second definitions. If we consider this code: u8 varA; u8 varB; u8 varC; void * pvar=varB; *pvar = ioread8(valid_memory); Depending if ioread8 returns a u8 or a unsigned int, aren't we also accessing varC? Could not this be a problem? If I decide to send a patch and fix it, is there a clever script that I can run on my x86 computer to test if the patch works on all arches? Thanks -- Ricardo Ribalda -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: Can this be a invalid memory access? (was: Re: [PATCH] spi/xilinx: Cast ioread32/iowrite32 function pointers)
Hi Ricardo,
On Mon, Feb 2, 2015 at 3:05 PM, Ricardo Ribalda Delgado
ricardo.riba...@gmail.com wrote:
On Mon, Feb 2, 2015 at 2:04 PM, Geert Uytterhoeven ge...@linux-m68k.org
wrote:
void * pvar=varB;
... = varB;
*pvar = ioread8(valid_memory);
Depending if ioread8 returns a u8 or a unsigned int, aren't we also
accessing varC?
Could not this be a problem?
Please try to compile the above.
The compiler will tell you you cannot dereference a void pointer.
Now, replace void * by unsigned int *.
After that, varC will indeed be overwritten. But the compiler will still have
warned you that you assigned the address of an u8 variable to an
unsigned int pointer.
I am still a bit worried, that the same function has different
prototypes and implementations depending on the arch. But
unfortunately I cannot make a counter-example that can cause an
error.
So far, the closest I have get is this:
ricardo@neopili:/tmp$ cat arch.c
#include stdint.h
You should include io.h here...
unsigned int my_ioread8(){
... then the compiler will complain here.
return 0xdeadbeef;
}
void test_read();
int main(int argc, char *argv[]){
test_read();
return 0;
}
ricardo@neopili:/tmp$ cat io.h
#ifndef IO_H
#define IO_H
#include stdint.h
uint8_t my_ioread8();
#endif
ricardo@neopili:/tmp$ cat driver.c
#include io.h
#include stdint.h
#include stdio.h
void test_read(){
uint8_t varA=0x1;
uint8_t varB=0x2;
uint8_t varC=0x3;
varB = my_ioread8();
fprintf(stdout, varA=%d varB=%d varC=%d\n,
varA, varB, varC);
}
It does not produce any error at build time:
ricardo@neopili:/tmp$ make
cc -Wall -c -o arch.o arch.c
cc -Wall -c -o driver.o driver.c
cc -Wall arch.o driver.o -o test
and it works fine on amd86
ricardo@neopili:/tmp$ ./test
varA=1 varB=239 varC=3
But in the hypothetical case where the calling convention will be
different for unsigned int and u8, there will be an issue
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say programmer or something like that.
-- Linus Torvalds
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: Can this be a invalid memory access? (was: Re: [PATCH] spi/xilinx: Cast ioread32/iowrite32 function pointers)
Hello Geert
On Mon, Feb 2, 2015 at 2:04 PM, Geert Uytterhoeven ge...@linux-m68k.org wrote:
void * pvar=varB;
... = varB;
*pvar = ioread8(valid_memory);
Depending if ioread8 returns a u8 or a unsigned int, aren't we also
accessing varC?
Could not this be a problem?
Please try to compile the above.
The compiler will tell you you cannot dereference a void pointer.
Now, replace void * by unsigned int *.
After that, varC will indeed be overwritten. But the compiler will still have
warned you that you assigned the address of an u8 variable to an
unsigned int pointer.
I am still a bit worried, that the same function has different
prototypes and implementations depending on the arch. But
unfortunately I cannot make a counter-example that can cause an
error.
So far, the closest I have get is this:
ricardo@neopili:/tmp$ cat arch.c
#include stdint.h
unsigned int my_ioread8(){
return 0xdeadbeef;
}
void test_read();
int main(int argc, char *argv[]){
test_read();
return 0;
}
ricardo@neopili:/tmp$ cat io.h
#ifndef IO_H
#define IO_H
#include stdint.h
uint8_t my_ioread8();
#endif
ricardo@neopili:/tmp$ cat driver.c
#include io.h
#include stdint.h
#include stdio.h
void test_read(){
uint8_t varA=0x1;
uint8_t varB=0x2;
uint8_t varC=0x3;
varB = my_ioread8();
fprintf(stdout, varA=%d varB=%d varC=%d\n,
varA, varB, varC);
}
It does not produce any error at build time:
ricardo@neopili:/tmp$ make
cc -Wall -c -o arch.o arch.c
cc -Wall -c -o driver.o driver.c
cc -Wall arch.o driver.o -o test
and it works fine on amd86
ricardo@neopili:/tmp$ ./test
varA=1 varB=239 varC=3
But in the hypothetical case where the calling convention will be
different for unsigned int and u8, there will be an issue
The reason that I am interested in this is that I want to be able to
have code like:
struct priv_data{
u16 (read *)(void __iomem *);
};
endian=detect_endianness()
if (endian == BIG)
priv-read= ioread16be;
else
priv-read= ioread16;
data = priv-read(iomem+DATA);
Unfortunately, with the current code, this does not work on all the
arches, and the workaround is having wrappers for ioread and iowrite
like in drivers/spi/spi-xilinx.c
static void xspi_write32(u32 val, void __iomem *addr)
{
iowrite32(val, addr);
}
static void xspi_write32_be(u32 val, void __iomem *addr)
{
iowrite32be(val, addr);
}
Regards
--
Ricardo Ribalda
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Can this be a invalid memory access? (was: Re: [PATCH] spi/xilinx: Cast ioread32/iowrite32 function pointers)
Hello Regarding ioread8 et al. On include/asm-generic/io.h is defined as: extern unsigned int ioread8(void __iomem *); On include/asm-generic/io.h: static inline u8 ioread8(const volatile void __iomem *addr) Please ignore the qualifiers right now. The first function returns an unsigned integer, the second a u8. Through #ifdefs, different arches uses the first or the second definitions. If we consider this code: u8 varA; u8 varB; u8 varC; void * pvar=varB; *pvar = ioread8(valid_memory); Depending if ioread8 returns a u8 or a unsigned int, aren't we also accessing varC? Could not this be a problem? If I decide to send a patch and fix it, is there a clever script that I can run on my x86 computer to test if the patch works on all arches? Thanks -- Ricardo Ribalda -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: Can this be a invalid memory access? (was: Re: [PATCH] spi/xilinx: Cast ioread32/iowrite32 function pointers)
Hi Ricardo,
On Mon, Feb 2, 2015 at 1:49 PM, Ricardo Ribalda Delgado
ricardo.riba...@gmail.com wrote:
Regarding ioread8 et al.
On include/asm-generic/io.h is defined as:
extern unsigned int ioread8(void __iomem *);
On include/asm-generic/io.h:
static inline u8 ioread8(const volatile void __iomem *addr)
Please ignore the qualifiers right now. The first function returns an
unsigned integer, the second a u8.
Through #ifdefs, different arches uses the first or the second definitions.
If we consider this code:
u8 varA;
u8 varB;
u8 varC;
void * pvar=varB;
... = varB;
*pvar = ioread8(valid_memory);
Depending if ioread8 returns a u8 or a unsigned int, aren't we also
accessing varC?
Could not this be a problem?
Please try to compile the above.
The compiler will tell you you cannot dereference a void pointer.
Now, replace void * by unsigned int *.
After that, varC will indeed be overwritten. But the compiler will still have
warned you that you assigned the address of an u8 variable to an
unsigned int pointer.
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say programmer or something like that.
-- Linus Torvalds
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
