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 malat@debian.org Cc: stable@vger.kernel.org # v4.19+ Cc: Bartlomiej Zolnierkiewicz b.zolnierkie@samsung.com 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;
On Mon, Jan 7, 2019 at 6:31 PM Rob Herring robh@kernel.org 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 malat@debian.org
Thanks
Reported-by: Mathieu Malaterre malat@debian.org Cc: stable@vger.kernel.org # v4.19+ Cc: Bartlomiej Zolnierkiewicz b.zolnierkie@samsung.com 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
Hi Rob,
On Mon, Jan 7, 2019 at 9:11 PM Mathieu Malaterre malat@debian.org wrote:
On Mon, Jan 7, 2019 at 6:31 PM Rob Herring robh@kernel.org 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 malat@debian.org
Just for curiosity, why would you keep:
if (strcmp(dp->name, "valkyrie") == 0)
Reported-by: Mathieu Malaterre malat@debian.org Cc: stable@vger.kernel.org # v4.19+ Cc: Bartlomiej Zolnierkiewicz b.zolnierkie@samsung.com 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
On Mon, Jan 7, 2019 at 2:36 PM Mathieu Malaterre malat@debian.org wrote:
Hi Rob,
On Mon, Jan 7, 2019 at 9:11 PM Mathieu Malaterre malat@debian.org wrote:
On Mon, Jan 7, 2019 at 6:31 PM Rob Herring robh@kernel.org 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 malat@debian.org
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
On 01/07/2019 09:11 PM, Mathieu Malaterre wrote:
On Mon, Jan 7, 2019 at 6:31 PM Rob Herring robh@kernel.org 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 malat@debian.org
Patch queued for 5.0, thanks.
Best regards, -- Bartlomiej Zolnierkiewicz Samsung R&D Institute Poland Samsung Electronics
linux-stable-mirror@lists.linaro.org