[PATCH] fbdev: offb: Fix OF node name handling

Subsystems: framebuffer layer, the rest

STALE2767d

5 messages, 3 authors, 2019-01-11 · open the first message on its own page

[PATCH] fbdev: offb: Fix OF node name handling

From: Rob Herring <robh@kernel.org>
Date: 2019-01-07 17:31:37

Commit 5c63e407aaab ("fbdev: Convert to using %pOFn instead of
device_node.name") changed how the OF FB driver handles the OF node
name. This missed the case where the node name is passed to
offb_init_palette_hacks(). This results in a NULL ptr dereference
in strncmp and breaks any system except ones using bootx with no display
node.

Fix this by making offb_init_palette_hacks() use the OF node pointer and
use of_node_name_prefix() helper function instead for node name
comparisons. This helps in moving all OF node name accesses to helper
functions in preparation to remove struct device_node.name pointer.

Fixes: 5c63e407aaab ("fbdev: Convert to using %pOFn instead of device_node.name")

Reported-by: Mathieu Malaterre <redacted>
Cc: stable@vger.kernel.org # v4.19+
Cc: Bartlomiej Zolnierkiewicz <redacted>
Cc: dri-devel@lists.freedesktop.org
Cc: linux-fbdev@vger.kernel.org
Signed-off-by: Rob Herring <robh@kernel.org>
---
 drivers/video/fbdev/offb.c | 18 +++++++++---------
 1 file changed, 9 insertions(+), 9 deletions(-)
diff --git a/drivers/video/fbdev/offb.c b/drivers/video/fbdev/offb.c
index 31f769d67195..057d3cdef92e 100644
--- a/drivers/video/fbdev/offb.c
+++ b/drivers/video/fbdev/offb.c
@@ -318,28 +318,28 @@ static void __iomem *offb_map_reg(struct device_node *np, int index,
 }
 
 static void offb_init_palette_hacks(struct fb_info *info, struct device_node *dp,
-				    const char *name, unsigned long address)
+				    unsigned long address)
 {
 	struct offb_par *par = (struct offb_par *) info->par;
 
-	if (dp && !strncmp(name, "ATY,Rage128", 11)) {
+	if (of_node_name_prefix(dp, "ATY,Rage128")) {
 		par->cmap_adr = offb_map_reg(dp, 2, 0, 0x1fff);
 		if (par->cmap_adr)
 			par->cmap_type = cmap_r128;
-	} else if (dp && (!strncmp(name, "ATY,RageM3pA", 12)
-			  || !strncmp(name, "ATY,RageM3p12A", 14))) {
+	} else if (of_node_name_prefix(dp, "ATY,RageM3pA") ||
+		   of_node_name_prefix(dp, "ATY,RageM3p12A")) {
 		par->cmap_adr = offb_map_reg(dp, 2, 0, 0x1fff);
 		if (par->cmap_adr)
 			par->cmap_type = cmap_M3A;
-	} else if (dp && !strncmp(name, "ATY,RageM3pB", 12)) {
+	} else if (of_node_name_prefix(dp, "ATY,RageM3pB")) {
 		par->cmap_adr = offb_map_reg(dp, 2, 0, 0x1fff);
 		if (par->cmap_adr)
 			par->cmap_type = cmap_M3B;
-	} else if (dp && !strncmp(name, "ATY,Rage6", 9)) {
+	} else if (of_node_name_prefix(dp, "ATY,Rage6")) {
 		par->cmap_adr = offb_map_reg(dp, 1, 0, 0x1fff);
 		if (par->cmap_adr)
 			par->cmap_type = cmap_radeon;
-	} else if (!strncmp(name, "ATY,", 4)) {
+	} else if (of_node_name_prefix(dp, "ATY,")) {
 		unsigned long base = address & 0xff000000UL;
 		par->cmap_adr  			ioremap(base + 0x7ff000, 0x1000) + 0xcc0;
@@ -350,7 +350,7 @@ static void offb_init_palette_hacks(struct fb_info *info, struct device_node *dp
 		par->cmap_adr = offb_map_reg(dp, 0, 0x6000, 0x1000);
 		if (par->cmap_adr)
 			par->cmap_type = cmap_gxt2000;
-	} else if (dp && !strncmp(name, "vga,Display-", 12)) {
+	} else if (of_node_name_prefix(dp, "vga,Display-")) {
 		/* Look for AVIVO initialized by SLOF */
 		struct device_node *pciparent = of_get_parent(dp);
 		const u32 *vid, *did;
@@ -438,7 +438,7 @@ static void __init offb_init_fb(const char *name,
 
 	par->cmap_type = cmap_unknown;
 	if (depth = 8)
-		offb_init_palette_hacks(info, dp, name, address);
+		offb_init_palette_hacks(info, dp, address);
 	else
 		fix->visual = FB_VISUAL_TRUECOLOR;
 
-- 
2.19.1

Re: [PATCH] fbdev: offb: Fix OF node name handling

From: Mathieu Malaterre <hidden>
Date: 2019-01-07 20:11:48

On Mon, Jan 7, 2019 at 6:31 PM Rob Herring [off-list ref] wrote:
Commit 5c63e407aaab ("fbdev: Convert to using %pOFn instead of
device_node.name") changed how the OF FB driver handles the OF node
name. This missed the case where the node name is passed to
offb_init_palette_hacks(). This results in a NULL ptr dereference
in strncmp and breaks any system except ones using bootx with no display
node.

Fix this by making offb_init_palette_hacks() use the OF node pointer and
use of_node_name_prefix() helper function instead for node name
comparisons. This helps in moving all OF node name accesses to helper
functions in preparation to remove struct device_node.name pointer.

Fixes: 5c63e407aaab ("fbdev: Convert to using %pOFn instead of device_node.name")
Looks good to me:

...
[    0.000000] Linux version 5.0.0-rc1+ (malat@debian.org) (gcc
version 6.3.0 20170516 (Debian 6.3.0-18)) #21 Mon Jan 7 21:03:53 CET
2019
...

So here is my :

Tested-by: Mathieu Malaterre <redacted>

Thanks
quoted hunk
Reported-by: Mathieu Malaterre <redacted>
Cc: stable@vger.kernel.org # v4.19+
Cc: Bartlomiej Zolnierkiewicz <redacted>
Cc: dri-devel@lists.freedesktop.org
Cc: linux-fbdev@vger.kernel.org
Signed-off-by: Rob Herring <robh@kernel.org>
---
 drivers/video/fbdev/offb.c | 18 +++++++++---------
 1 file changed, 9 insertions(+), 9 deletions(-)
diff --git a/drivers/video/fbdev/offb.c b/drivers/video/fbdev/offb.c
index 31f769d67195..057d3cdef92e 100644
--- a/drivers/video/fbdev/offb.c
+++ b/drivers/video/fbdev/offb.c
@@ -318,28 +318,28 @@ static void __iomem *offb_map_reg(struct device_node *np, int index,
 }

 static void offb_init_palette_hacks(struct fb_info *info, struct device_node *dp,
-                                   const char *name, unsigned long address)
+                                   unsigned long address)
 {
        struct offb_par *par = (struct offb_par *) info->par;

-       if (dp && !strncmp(name, "ATY,Rage128", 11)) {
+       if (of_node_name_prefix(dp, "ATY,Rage128")) {
                par->cmap_adr = offb_map_reg(dp, 2, 0, 0x1fff);
                if (par->cmap_adr)
                        par->cmap_type = cmap_r128;
-       } else if (dp && (!strncmp(name, "ATY,RageM3pA", 12)
-                         || !strncmp(name, "ATY,RageM3p12A", 14))) {
+       } else if (of_node_name_prefix(dp, "ATY,RageM3pA") ||
+                  of_node_name_prefix(dp, "ATY,RageM3p12A")) {
                par->cmap_adr = offb_map_reg(dp, 2, 0, 0x1fff);
                if (par->cmap_adr)
                        par->cmap_type = cmap_M3A;
-       } else if (dp && !strncmp(name, "ATY,RageM3pB", 12)) {
+       } else if (of_node_name_prefix(dp, "ATY,RageM3pB")) {
                par->cmap_adr = offb_map_reg(dp, 2, 0, 0x1fff);
                if (par->cmap_adr)
                        par->cmap_type = cmap_M3B;
-       } else if (dp && !strncmp(name, "ATY,Rage6", 9)) {
+       } else if (of_node_name_prefix(dp, "ATY,Rage6")) {
                par->cmap_adr = offb_map_reg(dp, 1, 0, 0x1fff);
                if (par->cmap_adr)
                        par->cmap_type = cmap_radeon;
-       } else if (!strncmp(name, "ATY,", 4)) {
+       } else if (of_node_name_prefix(dp, "ATY,")) {
                unsigned long base = address & 0xff000000UL;
                par->cmap_adr >                         ioremap(base + 0x7ff000, 0x1000) + 0xcc0;
@@ -350,7 +350,7 @@ static void offb_init_palette_hacks(struct fb_info *info, struct device_node *dp
                par->cmap_adr = offb_map_reg(dp, 0, 0x6000, 0x1000);
                if (par->cmap_adr)
                        par->cmap_type = cmap_gxt2000;
-       } else if (dp && !strncmp(name, "vga,Display-", 12)) {
+       } else if (of_node_name_prefix(dp, "vga,Display-")) {
                /* Look for AVIVO initialized by SLOF */
                struct device_node *pciparent = of_get_parent(dp);
                const u32 *vid, *did;
@@ -438,7 +438,7 @@ static void __init offb_init_fb(const char *name,

        par->cmap_type = cmap_unknown;
        if (depth = 8)
-               offb_init_palette_hacks(info, dp, name, address);
+               offb_init_palette_hacks(info, dp, address);
        else
                fix->visual = FB_VISUAL_TRUECOLOR;

--
2.19.1

Re: [PATCH] fbdev: offb: Fix OF node name handling

From: Mathieu Malaterre <hidden>
Date: 2019-01-07 20:36:41

Hi Rob,

On Mon, Jan 7, 2019 at 9:11 PM Mathieu Malaterre [off-list ref] wrote:
On Mon, Jan 7, 2019 at 6:31 PM Rob Herring [off-list ref] wrote:
quoted
Commit 5c63e407aaab ("fbdev: Convert to using %pOFn instead of
device_node.name") changed how the OF FB driver handles the OF node
name. This missed the case where the node name is passed to
offb_init_palette_hacks(). This results in a NULL ptr dereference
in strncmp and breaks any system except ones using bootx with no display
node.

Fix this by making offb_init_palette_hacks() use the OF node pointer and
use of_node_name_prefix() helper function instead for node name
comparisons. This helps in moving all OF node name accesses to helper
functions in preparation to remove struct device_node.name pointer.

Fixes: 5c63e407aaab ("fbdev: Convert to using %pOFn instead of device_node.name")
Looks good to me:

...
[    0.000000] Linux version 5.0.0-rc1+ (malat@debian.org) (gcc
version 6.3.0 20170516 (Debian 6.3.0-18)) #21 Mon Jan 7 21:03:53 CET
2019
...

So here is my :

Tested-by: Mathieu Malaterre <redacted>
Just for curiosity, why would you keep:

     if (strcmp(dp->name, "valkyrie") = 0)


quoted
Reported-by: Mathieu Malaterre <redacted>
Cc: stable@vger.kernel.org # v4.19+
Cc: Bartlomiej Zolnierkiewicz <redacted>
Cc: dri-devel@lists.freedesktop.org
Cc: linux-fbdev@vger.kernel.org
Signed-off-by: Rob Herring <robh@kernel.org>
---
 drivers/video/fbdev/offb.c | 18 +++++++++---------
 1 file changed, 9 insertions(+), 9 deletions(-)
diff --git a/drivers/video/fbdev/offb.c b/drivers/video/fbdev/offb.c
index 31f769d67195..057d3cdef92e 100644
--- a/drivers/video/fbdev/offb.c
+++ b/drivers/video/fbdev/offb.c
@@ -318,28 +318,28 @@ static void __iomem *offb_map_reg(struct device_node *np, int index,
 }

 static void offb_init_palette_hacks(struct fb_info *info, struct device_node *dp,
-                                   const char *name, unsigned long address)
+                                   unsigned long address)
 {
        struct offb_par *par = (struct offb_par *) info->par;

-       if (dp && !strncmp(name, "ATY,Rage128", 11)) {
+       if (of_node_name_prefix(dp, "ATY,Rage128")) {
                par->cmap_adr = offb_map_reg(dp, 2, 0, 0x1fff);
                if (par->cmap_adr)
                        par->cmap_type = cmap_r128;
-       } else if (dp && (!strncmp(name, "ATY,RageM3pA", 12)
-                         || !strncmp(name, "ATY,RageM3p12A", 14))) {
+       } else if (of_node_name_prefix(dp, "ATY,RageM3pA") ||
+                  of_node_name_prefix(dp, "ATY,RageM3p12A")) {
                par->cmap_adr = offb_map_reg(dp, 2, 0, 0x1fff);
                if (par->cmap_adr)
                        par->cmap_type = cmap_M3A;
-       } else if (dp && !strncmp(name, "ATY,RageM3pB", 12)) {
+       } else if (of_node_name_prefix(dp, "ATY,RageM3pB")) {
                par->cmap_adr = offb_map_reg(dp, 2, 0, 0x1fff);
                if (par->cmap_adr)
                        par->cmap_type = cmap_M3B;
-       } else if (dp && !strncmp(name, "ATY,Rage6", 9)) {
+       } else if (of_node_name_prefix(dp, "ATY,Rage6")) {
                par->cmap_adr = offb_map_reg(dp, 1, 0, 0x1fff);
                if (par->cmap_adr)
                        par->cmap_type = cmap_radeon;
-       } else if (!strncmp(name, "ATY,", 4)) {
+       } else if (of_node_name_prefix(dp, "ATY,")) {
                unsigned long base = address & 0xff000000UL;
                par->cmap_adr > >                         ioremap(base + 0x7ff000, 0x1000) + 0xcc0;
@@ -350,7 +350,7 @@ static void offb_init_palette_hacks(struct fb_info *info, struct device_node *dp
                par->cmap_adr = offb_map_reg(dp, 0, 0x6000, 0x1000);
                if (par->cmap_adr)
                        par->cmap_type = cmap_gxt2000;
-       } else if (dp && !strncmp(name, "vga,Display-", 12)) {
+       } else if (of_node_name_prefix(dp, "vga,Display-")) {
                /* Look for AVIVO initialized by SLOF */
                struct device_node *pciparent = of_get_parent(dp);
                const u32 *vid, *did;
@@ -438,7 +438,7 @@ static void __init offb_init_fb(const char *name,

        par->cmap_type = cmap_unknown;
        if (depth = 8)
-               offb_init_palette_hacks(info, dp, name, address);
+               offb_init_palette_hacks(info, dp, address);
        else
                fix->visual = FB_VISUAL_TRUECOLOR;

--
2.19.1

Re: [PATCH] fbdev: offb: Fix OF node name handling

From: Rob Herring <robh@kernel.org>
Date: 2019-01-07 23:51:16

On Mon, Jan 7, 2019 at 2:36 PM Mathieu Malaterre [off-list ref] wrote:
Hi Rob,

On Mon, Jan 7, 2019 at 9:11 PM Mathieu Malaterre [off-list ref] wrote:
quoted
On Mon, Jan 7, 2019 at 6:31 PM Rob Herring [off-list ref] wrote:
quoted
Commit 5c63e407aaab ("fbdev: Convert to using %pOFn instead of
device_node.name") changed how the OF FB driver handles the OF node
name. This missed the case where the node name is passed to
offb_init_palette_hacks(). This results in a NULL ptr dereference
in strncmp and breaks any system except ones using bootx with no display
node.

Fix this by making offb_init_palette_hacks() use the OF node pointer and
use of_node_name_prefix() helper function instead for node name
comparisons. This helps in moving all OF node name accesses to helper
functions in preparation to remove struct device_node.name pointer.

Fixes: 5c63e407aaab ("fbdev: Convert to using %pOFn instead of device_node.name")
Looks good to me:

...
[    0.000000] Linux version 5.0.0-rc1+ (malat@debian.org) (gcc
version 6.3.0 20170516 (Debian 6.3.0-18)) #21 Mon Jan 7 21:03:53 CET
2019
...

So here is my :

Tested-by: Mathieu Malaterre <redacted>
Just for curiosity, why would you keep:

     if (strcmp(dp->name, "valkyrie") = 0)
Mainly just because I already have another patch to do that which I
haven't sent out.

Rob

Re: [PATCH] fbdev: offb: Fix OF node name handling

From: Bartlomiej Zolnierkiewicz <hidden>
Date: 2019-01-11 12:53:45

On 01/07/2019 09:11 PM, Mathieu Malaterre wrote:
On Mon, Jan 7, 2019 at 6:31 PM Rob Herring [off-list ref] wrote:
quoted
Commit 5c63e407aaab ("fbdev: Convert to using %pOFn instead of
device_node.name") changed how the OF FB driver handles the OF node
name. This missed the case where the node name is passed to
offb_init_palette_hacks(). This results in a NULL ptr dereference
in strncmp and breaks any system except ones using bootx with no display
node.

Fix this by making offb_init_palette_hacks() use the OF node pointer and
use of_node_name_prefix() helper function instead for node name
comparisons. This helps in moving all OF node name accesses to helper
functions in preparation to remove struct device_node.name pointer.

Fixes: 5c63e407aaab ("fbdev: Convert to using %pOFn instead of device_node.name")
Looks good to me:

...
[    0.000000] Linux version 5.0.0-rc1+ (malat@debian.org) (gcc
version 6.3.0 20170516 (Debian 6.3.0-18)) #21 Mon Jan 7 21:03:53 CET
2019
...

So here is my :

Tested-by: Mathieu Malaterre <redacted>
Patch queued for 5.0, thanks.

Best regards,
--
Bartlomiej Zolnierkiewicz
Samsung R&D Institute Poland
Samsung Electronics
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help