All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH update] kexec/fs2dt.c: wrong dt node fix
@ 2016-02-26 11:57 Dave Young
  2016-02-26 13:39 ` Andrew Jones
  0 siblings, 1 reply; 3+ messages in thread
From: Dave Young @ 2016-02-26 11:57 UTC (permalink / raw)
  To: kexec; +Cc: Andrew Jones, Simon Horman

2nd kernel hangs early because of a regression caused by below commit:
commit 68262155d8c661586b809bc5301a7dff1c378137
Author: Andrew Jones <drjones@redhat.com>
Date:   Fri Nov 20 12:31:53 2015 -0500

    kexec/fs2dt: cleanup pathname
    
    putnode() will add the trailing '/', avoid having two. Also
    pathstart is unused, get rid of it.
    
    Signed-off-by: Andrew Jones <drjones@redhat.com>
    Signed-off-by: Simon Horman <horms@verge.net.au>

The actual purpose of the commit is to avoid double slash in pathname.
But unfortunately in function putnode() we have below magics to get the node
name:
	basename = strrchr(pathname,'/') + 1;
	...
	strcpy((void *)dt, *basename ? basename : "");
	...
	strcat(pathname, "/");

We treat none zero basename as a node name, then concat a slash to open the
directory for later property handling.
pathname originally was "/proc/device-tree/" so for the first run of putnode
it will cause double slashes. With the commit above mentioned there are no
double slashes but we will copy "device-tree" to dt. Thus kexec kernel is not
happy..

Instead let's fix it by only concating slash when the basenanme is not empty
and restore the initial value of pathname as "/proc/device-tree/"

Note: I only reproduce the issue with loading older kernel like 3.10 in RHEL. I do
not see the problem in new kernels in Fedora.

Signed-off-by: Dave Young <dyoung@redhat.com>
---
 kexec/fs2dt.c |    5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

--- kexec-tools.orig/kexec/fs2dt.c
+++ kexec-tools/kexec/fs2dt.c
@@ -577,7 +577,8 @@ static void putnode(void)
 	strcpy((void *)dt, *basename ? basename : "");
 	dt += ((plen + 4)/4);
 
-	strcat(pathname, "/");
+	if (*basename)
+		strcat(pathname, "/");
 	dn = pathname + strlen(pathname);
 
 	putprops(dn, namelist, numlist);
@@ -804,7 +805,7 @@ static void add_boot_block(char **bufp,
 
 void create_flatten_tree(char **bufp, off_t *sizep, const char *cmdline)
 {
-	strcpy(pathname, "/proc/device-tree");
+	strcpy(pathname, "/proc/device-tree/");
 
 	dt_cur_size = INIT_TREE_WORDS;
 	dt_base = malloc(dt_cur_size*4);

_______________________________________________
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH update] kexec/fs2dt.c: wrong dt node fix
  2016-02-26 11:57 [PATCH update] kexec/fs2dt.c: wrong dt node fix Dave Young
@ 2016-02-26 13:39 ` Andrew Jones
  2016-03-02  0:14   ` Simon Horman
  0 siblings, 1 reply; 3+ messages in thread
From: Andrew Jones @ 2016-02-26 13:39 UTC (permalink / raw)
  To: Dave Young; +Cc: Simon Horman, kexec

On Fri, Feb 26, 2016 at 07:57:55PM +0800, Dave Young wrote:
> 2nd kernel hangs early because of a regression caused by below commit:
> commit 68262155d8c661586b809bc5301a7dff1c378137
> Author: Andrew Jones <drjones@redhat.com>
> Date:   Fri Nov 20 12:31:53 2015 -0500
> 
>     kexec/fs2dt: cleanup pathname
>     
>     putnode() will add the trailing '/', avoid having two. Also
>     pathstart is unused, get rid of it.
>     
>     Signed-off-by: Andrew Jones <drjones@redhat.com>
>     Signed-off-by: Simon Horman <horms@verge.net.au>
> 
> The actual purpose of the commit is to avoid double slash in pathname.
> But unfortunately in function putnode() we have below magics to get the node
> name:
> 	basename = strrchr(pathname,'/') + 1;
> 	...
> 	strcpy((void *)dt, *basename ? basename : "");
> 	...
> 	strcat(pathname, "/");
> 
> We treat none zero basename as a node name, then concat a slash to open the
> directory for later property handling.
> pathname originally was "/proc/device-tree/" so for the first run of putnode
> it will cause double slashes. With the commit above mentioned there are no
> double slashes but we will copy "device-tree" to dt. Thus kexec kernel is not
> happy..
> 
> Instead let's fix it by only concating slash when the basenanme is not empty
> and restore the initial value of pathname as "/proc/device-tree/"
> 
> Note: I only reproduce the issue with loading older kernel like 3.10 in RHEL. I do
> not see the problem in new kernels in Fedora.
> 
> Signed-off-by: Dave Young <dyoung@redhat.com>

Err.. sorry I missed that strrchr() for the basename.

Reviewed-by: Andrew Jones <drjones@redhat.com>

> ---
>  kexec/fs2dt.c |    5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
> 
> --- kexec-tools.orig/kexec/fs2dt.c
> +++ kexec-tools/kexec/fs2dt.c
> @@ -577,7 +577,8 @@ static void putnode(void)
>  	strcpy((void *)dt, *basename ? basename : "");
>  	dt += ((plen + 4)/4);
>  
> -	strcat(pathname, "/");
> +	if (*basename)
> +		strcat(pathname, "/");
>  	dn = pathname + strlen(pathname);
>  
>  	putprops(dn, namelist, numlist);
> @@ -804,7 +805,7 @@ static void add_boot_block(char **bufp,
>  
>  void create_flatten_tree(char **bufp, off_t *sizep, const char *cmdline)
>  {
> -	strcpy(pathname, "/proc/device-tree");
> +	strcpy(pathname, "/proc/device-tree/");
>  
>  	dt_cur_size = INIT_TREE_WORDS;
>  	dt_base = malloc(dt_cur_size*4);

_______________________________________________
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH update] kexec/fs2dt.c: wrong dt node fix
  2016-02-26 13:39 ` Andrew Jones
@ 2016-03-02  0:14   ` Simon Horman
  0 siblings, 0 replies; 3+ messages in thread
From: Simon Horman @ 2016-03-02  0:14 UTC (permalink / raw)
  To: Andrew Jones; +Cc: kexec, Dave Young

On Fri, Feb 26, 2016 at 02:39:20PM +0100, Andrew Jones wrote:
> On Fri, Feb 26, 2016 at 07:57:55PM +0800, Dave Young wrote:
> > 2nd kernel hangs early because of a regression caused by below commit:
> > commit 68262155d8c661586b809bc5301a7dff1c378137
> > Author: Andrew Jones <drjones@redhat.com>
> > Date:   Fri Nov 20 12:31:53 2015 -0500
> > 
> >     kexec/fs2dt: cleanup pathname
> >     
> >     putnode() will add the trailing '/', avoid having two. Also
> >     pathstart is unused, get rid of it.
> >     
> >     Signed-off-by: Andrew Jones <drjones@redhat.com>
> >     Signed-off-by: Simon Horman <horms@verge.net.au>
> > 
> > The actual purpose of the commit is to avoid double slash in pathname.
> > But unfortunately in function putnode() we have below magics to get the node
> > name:
> > 	basename = strrchr(pathname,'/') + 1;
> > 	...
> > 	strcpy((void *)dt, *basename ? basename : "");
> > 	...
> > 	strcat(pathname, "/");
> > 
> > We treat none zero basename as a node name, then concat a slash to open the
> > directory for later property handling.
> > pathname originally was "/proc/device-tree/" so for the first run of putnode
> > it will cause double slashes. With the commit above mentioned there are no
> > double slashes but we will copy "device-tree" to dt. Thus kexec kernel is not
> > happy..
> > 
> > Instead let's fix it by only concating slash when the basenanme is not empty
> > and restore the initial value of pathname as "/proc/device-tree/"
> > 
> > Note: I only reproduce the issue with loading older kernel like 3.10 in RHEL. I do
> > not see the problem in new kernels in Fedora.
> > 
> > Signed-off-by: Dave Young <dyoung@redhat.com>
> 
> Err.. sorry I missed that strrchr() for the basename.
> 
> Reviewed-by: Andrew Jones <drjones@redhat.com>

Thanks, applied.

_______________________________________________
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2016-03-02  0:15 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-02-26 11:57 [PATCH update] kexec/fs2dt.c: wrong dt node fix Dave Young
2016-02-26 13:39 ` Andrew Jones
2016-03-02  0:14   ` Simon Horman

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.