Re: [PATCH] powerpc/prom: Avoid reference to potentially freed memory

[Michael Ellerman]

On Fri, 2015-10-16 at 22:09 +0200, Christophe JAILLET wrote:

> Le 16/10/2015 12:02, Michael Ellerman a écrit :

> > As the kbuild robot detected you have left an extra "}" here.
> > 
> > I don't mind too much if you send patches that aren't compile tested, but 
> > you
> > might save yourself some time by compiling them.
> 
> Sorry about it, and thanks for your patience.
> IMHO, this should never happen and patches should be at least 
> compile-tested.

Preferably yes. But for these sort of cleanup patches, where you're touching
lots of different arches, I realise it can be tedious to find all the various
cross compilers. So I'm willing to cut you a bit of slack :)

> I will be more careful and compile-test any new patch I submit.

Thanks.

cheers

___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev

Re: [PATCH] powerpc/prom: Avoid reference to potentially freed memory

[Christophe JAILLET]

Le 16/10/2015 12:02, Michael Ellerman a écrit :

As the kbuild robot detected you have left an extra "}" here.

I don't mind too much if you send patches that aren't compile tested, but you
might save yourself some time by compiling them.


Sorry about it, and thanks for your patience.
IMHO, this should never happen and patches should be at least 
compile-tested.


I will be more careful and compile-test any new patch I submit.

CJ

___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev

Re: [PATCH] powerpc/prom: Avoid reference to potentially freed memory

[kbuild test robot]

Hi Christophe,

[auto build test ERROR on powerpc/next -- if it's inappropriate base, please 
suggest rules for selecting the more suitable base]

url:
https://github.com/0day-ci/linux/commits/Christophe-JAILLET/powerpc-prom-Avoid-reference-to-potentially-freed-memory/20151016-141714
config: powerpc-defconfig (attached as .config)
reproduce:
wget 
https://git.kernel.org/cgit/linux/kernel/git/wfg/lkp-tests.git/plain/sbin/make.cross
 -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# save the attached .config to linux build tree
make.cross ARCH=powerpc 

All errors (new ones prefixed by >>):

   arch/powerpc/kernel/prom.c: In function 'of_get_ibm_chip_id':
>> arch/powerpc/kernel/prom.c:787:23: error: unused variable 'old' 
>> [-Werror=unused-variable]
  struct device_node *old = np;
  ^
>> arch/powerpc/kernel/prom.c:795:15: error: 'old' undeclared (first use in 
>> this function)
  of_node_put(old);
  ^
   arch/powerpc/kernel/prom.c:795:15: note: each undeclared identifier is 
reported only once for each function it appears in
   arch/powerpc/kernel/prom.c: At top level:
>> arch/powerpc/kernel/prom.c:797:2: error: expected identifier or '(' before 
>> 'return'
 return -1;
 ^
>> arch/powerpc/kernel/prom.c:798:1: error: expected identifier or '(' before 
>> '}' token
}
^
   arch/powerpc/kernel/prom.c: In function 'of_get_ibm_chip_id':
   arch/powerpc/kernel/prom.c:796:2: error: control reaches end of non-void 
function [-Werror=return-type]
 }
 ^
   cc1: all warnings being treated as errors

vim +/old +787 arch/powerpc/kernel/prom.c

b37193b7 Benjamin Herrenschmidt 2013-07-15  781   * be found.
b37193b7 Benjamin Herrenschmidt 2013-07-15  782   */
b37193b7 Benjamin Herrenschmidt 2013-07-15  783  int of_get_ibm_chip_id(struct 
device_node *np)
b37193b7 Benjamin Herrenschmidt 2013-07-15  784  {
b37193b7 Benjamin Herrenschmidt 2013-07-15  785 of_node_get(np);
b37193b7 Benjamin Herrenschmidt 2013-07-15  786 while (np) {
b37193b7 Benjamin Herrenschmidt 2013-07-15 @787 struct 
device_node *old = np;
12540384 Christophe JAILLET 2015-10-16  788 u32 chip_id;
b37193b7 Benjamin Herrenschmidt 2013-07-15  789  
12540384 Christophe JAILLET 2015-10-16  790 if 
(!of_property_read_u32(np, "ibm,chip-id", _id))
b37193b7 Benjamin Herrenschmidt 2013-07-15  791 
of_node_put(np);
12540384 Christophe JAILLET 2015-10-16  792 return 
chip_id;
b37193b7 Benjamin Herrenschmidt 2013-07-15  793 }
b37193b7 Benjamin Herrenschmidt 2013-07-15  794 np = 
of_get_parent(np);
b37193b7 Benjamin Herrenschmidt 2013-07-15 @795 
of_node_put(old);
b37193b7 Benjamin Herrenschmidt 2013-07-15  796 }
b37193b7 Benjamin Herrenschmidt 2013-07-15 @797 return -1;
b37193b7 Benjamin Herrenschmidt 2013-07-15 @798  }
b130e7c0 Dan Streetman  2015-05-07  799  
EXPORT_SYMBOL(of_get_ibm_chip_id);
b37193b7 Benjamin Herrenschmidt 2013-07-15  800  
3eb906c6 Michael Ellerman   2013-11-20  801  /**

:: The code at line 787 was first introduced by commit
:: b37193b71846858d816e152d3a5db010d7b73f5e powerpc/powernv: Add helper to 
get ibm,chip-id of a node

:: TO: Benjamin Herrenschmidt 
:: CC: Benjamin Herrenschmidt 

---
0-DAY kernel test infrastructureOpen Source Technology Center
https://lists.01.org/pipermail/kbuild-all   Intel Corporation


.config.gz
Description: Binary data
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev

[PATCH] powerpc/prom: Avoid reference to potentially freed memory

[Christophe JAILLET]

of_get_property() is used inside the loop, but then the reference to the
node is dropped before dereferencing the prop pointer, which could by then
point to junk if the node has been freed.

Instead use of_property_read_u32() to actually read the property
value before dropping the reference.

Signed-off-by: Christophe JAILLET 
---
*** UNTESTED ***
---
 arch/powerpc/kernel/prom.c | 9 -
 1 file changed, 4 insertions(+), 5 deletions(-)

diff --git a/arch/powerpc/kernel/prom.c b/arch/powerpc/kernel/prom.c
index bef76c5..dc4f6a4 100644
--- a/arch/powerpc/kernel/prom.c
+++ b/arch/powerpc/kernel/prom.c
@@ -783,14 +783,13 @@ void __init early_get_first_memblock_info(void *params, 
phys_addr_t *size)
 int of_get_ibm_chip_id(struct device_node *np)
 {
of_node_get(np);
-   while(np) {
+   while (np) {
struct device_node *old = np;
-   const __be32 *prop;
+   u32 chip_id;
 
-   prop = of_get_property(np, "ibm,chip-id", NULL);
-   if (prop) {
+   if (!of_property_read_u32(np, "ibm,chip-id", _id))
of_node_put(np);
-   return be32_to_cpup(prop);
+   return chip_id;
}
np = of_get_parent(np);
of_node_put(old);
-- 
2.1.4

___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev

Re: [PATCH] powerpc/prom: Avoid reference to potentially freed memory

[Michael Ellerman]

On Fri, 2015-10-16 at 08:14 +0200, Christophe JAILLET wrote:

> of_get_property() is used inside the loop, but then the reference to the
> node is dropped before dereferencing the prop pointer, which could by then
> point to junk if the node has been freed.
> 
> Instead use of_property_read_u32() to actually read the property
> value before dropping the reference.
> 
> Signed-off-by: Christophe JAILLET 
> ---
> *** UNTESTED ***
> ---
>  arch/powerpc/kernel/prom.c | 9 -
>  1 file changed, 4 insertions(+), 5 deletions(-)
> 
> diff --git a/arch/powerpc/kernel/prom.c b/arch/powerpc/kernel/prom.c
> index bef76c5..dc4f6a4 100644
> --- a/arch/powerpc/kernel/prom.c
> +++ b/arch/powerpc/kernel/prom.c
> @@ -783,14 +783,13 @@ void __init early_get_first_memblock_info(void *params, 
> phys_addr_t *size)
>  int of_get_ibm_chip_id(struct device_node *np)
>  {
>   of_node_get(np);
> - while(np) {
> + while (np) {
>   struct device_node *old = np;
> - const __be32 *prop;
> + u32 chip_id;
>  
> - prop = of_get_property(np, "ibm,chip-id", NULL);
> - if (prop) {
> + if (!of_property_read_u32(np, "ibm,chip-id", _id))
>   of_node_put(np);
> - return be32_to_cpup(prop);
> + return chip_id;
>   }


As the kbuild robot detected you have left an extra "}" here.

I don't mind too much if you send patches that aren't compile tested, but you
might save yourself some time by compiling them.

There are x86->powerpc cross compilers here:

https://www.kernel.org/pub/tools/crosstool/files/bin/x86_64/4.9.0/x86_64-gcc-4.9.0-nolibc_powerpc64-linux.tar.gz

Or if you're running on Ubuntu you can just do:

$ apt-get install gcc-powerpc-linux-gnu

I think there's a package for Fedora too but I don't know the name off the top
of my head.

cheers

___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev