All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] initramfs: clean old path before creating a hardlink
@ 2018-11-16  7:12 Li Zhijian
  2018-11-22  1:55 ` Li Zhijian
  2018-11-26 23:34 ` Andrew Morton
  0 siblings, 2 replies; 4+ messages in thread
From: Li Zhijian @ 2018-11-16  7:12 UTC (permalink / raw)
  To: linux-kernel, Dominik Brodowski, Andrew Morton
  Cc: zhijianx.li, Li Zhijian, Philip Li

Previously, sys_link() will fail due to the new path is already existed.
this case ofen appears when we use a concated initrd, below is an
sample:

1) prepare a basic rootfs, it contains a regular files rc.local
lizhijian@:~/yocto-tiny-i386-2016-04-22$ cat etc/rc.local
 #!/bin/sh
 echo "Running /etc/rc.local..."
yocto-tiny-i386-2016-04-22$ find . | sed 's,^\./,,' | cpio -o -H newc | gzip -n -9 >../rootfs.cgz

2) create a extra initrd which also includes a etc/rc.local
lizhijian@:~/lkp-x86_64/etc$ echo "append initrd" >rc.local
lizhijian@:~/lkp/lkp-x86_64/etc$ cat rc.local
append initrd
lizhijian@:~/lkp/lkp-x86_64/etc$ ln rc.local rc.local.hardlink
append initrd
lizhijian@:~/lkp/lkp-x86_64/etc$ stat rc.local rc.local.hardlink
  File: 'rc.local'
  Size: 14        	Blocks: 8          IO Block: 4096   regular file
Device: 801h/2049d	Inode: 11296086    Links: 2
Access: (0664/-rw-rw-r--)  Uid: ( 1002/lizhijian)   Gid: ( 1002/lizhijian)
Access: 2018-11-15 16:08:28.654464815 +0800
Modify: 2018-11-15 16:07:57.514903210 +0800
Change: 2018-11-15 16:08:24.180228872 +0800
 Birth: -
  File: 'rc.local.hardlink'
  Size: 14        	Blocks: 8          IO Block: 4096   regular file
Device: 801h/2049d	Inode: 11296086    Links: 2
Access: (0664/-rw-rw-r--)  Uid: ( 1002/lizhijian)   Gid: ( 1002/lizhijian)
Access: 2018-11-15 16:08:28.654464815 +0800
Modify: 2018-11-15 16:07:57.514903210 +0800
Change: 2018-11-15 16:08:24.180228872 +0800
 Birth: -

lizhijian@:~/lkp/lkp-x86_64$ find . | sed 's,^\./,,' | cpio -o -H newc | gzip -n -9 >../rc-local.cgz
lizhijian@:~/lkp/lkp-x86_64$ gzip -dc ../rc-local.cgz | cpio -t
.
etc
etc/rc.local.hardlink <<< it will be extracted first at this initrd
etc/rc.local

3) concate 2 initrds and boot
lizhijian@:~/lkp$ cat rootfs.cgz rc-local.cgz >concate-initrd.cgz
lizhijian@:~/lkp$ qemu-system-x86_64 -nographic -enable-kvm -cpu host -smp 1 -m 1024 -kernel ~/lkp/linux/arch/x86/boot/bzImage -append "console=ttyS0 earlyprint=ttyS0 ignore_loglevel" -initrd ./concate-initr.cgz -serial stdio -nodefaults

In this case, sys_link(2) will fail and return -EEXIST, so we can only
get the rc.local at rootfs.cgz instead of rc-local.cgz

CC: Philip Li <philip.li@intel.com>
Signed-off-by: Li Zhijian <lizhijian@cn.fujitsu.com>
---
 init/initramfs.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/init/initramfs.c b/init/initramfs.c
index 6405577..381fd4c 100644
--- a/init/initramfs.c
+++ b/init/initramfs.c
@@ -291,12 +291,16 @@ static int __init do_reset(void)
 	return 1;
 }
 
+static void __init clean_path(char *path, umode_t fmode);
+
 static int __init maybe_link(void)
 {
 	if (nlink >= 2) {
 		char *old = find_link(major, minor, ino, mode, collected);
-		if (old)
+		if (old) {
+			clean_path(collected, 0);
 			return (ksys_link(old, collected) < 0) ? -1 : 1;
+		}
 	}
 	return 0;
 }
-- 
2.7.4


^ permalink raw reply related	[flat|nested] 4+ messages in thread
* Re: [PATCH] initramfs: clean old path before creating a hardlink
  2018-11-16  7:12 [PATCH] initramfs: clean old path before creating a hardlink Li Zhijian
@ 2018-11-22  1:55 ` Li Zhijian
  2018-11-26 23:34 ` Andrew Morton
  1 sibling, 0 replies; 4+ messages in thread
From: Li Zhijian @ 2018-11-22  1:55 UTC (permalink / raw)
  To: linux-kernel, Dominik Brodowski, Andrew Morton; +Cc: zhijianx.li, Philip Li

ping

This patch is to fix initrd cannot be extracted properly in some cases.


Thanks


On 11/16/2018 03:12 PM, Li Zhijian wrote:
> Previously, sys_link() will fail due to the new path is already existed.
> this case ofen appears when we use a concated initrd, below is an
> sample:
>
> 1) prepare a basic rootfs, it contains a regular files rc.local
> lizhijian@:~/yocto-tiny-i386-2016-04-22$ cat etc/rc.local
>   #!/bin/sh
>   echo "Running /etc/rc.local..."
> yocto-tiny-i386-2016-04-22$ find . | sed 's,^\./,,' | cpio -o -H newc | gzip -n -9 >../rootfs.cgz
>
> 2) create a extra initrd which also includes a etc/rc.local
> lizhijian@:~/lkp-x86_64/etc$ echo "append initrd" >rc.local
> lizhijian@:~/lkp/lkp-x86_64/etc$ cat rc.local
> append initrd
> lizhijian@:~/lkp/lkp-x86_64/etc$ ln rc.local rc.local.hardlink
> append initrd
> lizhijian@:~/lkp/lkp-x86_64/etc$ stat rc.local rc.local.hardlink
>    File: 'rc.local'
>    Size: 14        	Blocks: 8          IO Block: 4096   regular file
> Device: 801h/2049d	Inode: 11296086    Links: 2
> Access: (0664/-rw-rw-r--)  Uid: ( 1002/lizhijian)   Gid: ( 1002/lizhijian)
> Access: 2018-11-15 16:08:28.654464815 +0800
> Modify: 2018-11-15 16:07:57.514903210 +0800
> Change: 2018-11-15 16:08:24.180228872 +0800
>   Birth: -
>    File: 'rc.local.hardlink'
>    Size: 14        	Blocks: 8          IO Block: 4096   regular file
> Device: 801h/2049d	Inode: 11296086    Links: 2
> Access: (0664/-rw-rw-r--)  Uid: ( 1002/lizhijian)   Gid: ( 1002/lizhijian)
> Access: 2018-11-15 16:08:28.654464815 +0800
> Modify: 2018-11-15 16:07:57.514903210 +0800
> Change: 2018-11-15 16:08:24.180228872 +0800
>   Birth: -
>
> lizhijian@:~/lkp/lkp-x86_64$ find . | sed 's,^\./,,' | cpio -o -H newc | gzip -n -9 >../rc-local.cgz
> lizhijian@:~/lkp/lkp-x86_64$ gzip -dc ../rc-local.cgz | cpio -t
> .
> etc
> etc/rc.local.hardlink <<< it will be extracted first at this initrd
> etc/rc.local
>
> 3) concate 2 initrds and boot
> lizhijian@:~/lkp$ cat rootfs.cgz rc-local.cgz >concate-initrd.cgz
> lizhijian@:~/lkp$ qemu-system-x86_64 -nographic -enable-kvm -cpu host -smp 1 -m 1024 -kernel ~/lkp/linux/arch/x86/boot/bzImage -append "console=ttyS0 earlyprint=ttyS0 ignore_loglevel" -initrd ./concate-initr.cgz -serial stdio -nodefaults
>
> In this case, sys_link(2) will fail and return -EEXIST, so we can only
> get the rc.local at rootfs.cgz instead of rc-local.cgz
>
> CC: Philip Li <philip.li@intel.com>
> Signed-off-by: Li Zhijian <lizhijian@cn.fujitsu.com>
> ---
>   init/initramfs.c | 6 +++++-
>   1 file changed, 5 insertions(+), 1 deletion(-)
>
> diff --git a/init/initramfs.c b/init/initramfs.c
> index 6405577..381fd4c 100644
> --- a/init/initramfs.c
> +++ b/init/initramfs.c
> @@ -291,12 +291,16 @@ static int __init do_reset(void)
>   	return 1;
>   }
>   
> +static void __init clean_path(char *path, umode_t fmode);
> +
>   static int __init maybe_link(void)
>   {
>   	if (nlink >= 2) {
>   		char *old = find_link(major, minor, ino, mode, collected);
> -		if (old)
> +		if (old) {
> +			clean_path(collected, 0);
>   			return (ksys_link(old, collected) < 0) ? -1 : 1;
> +		}
>   	}
>   	return 0;
>   }



^ permalink raw reply	[flat|nested] 4+ messages in thread
* Re: [PATCH] initramfs: clean old path before creating a hardlink
  2018-11-16  7:12 [PATCH] initramfs: clean old path before creating a hardlink Li Zhijian
  2018-11-22  1:55 ` Li Zhijian
@ 2018-11-26 23:34 ` Andrew Morton
  2018-11-27  4:04   ` Li Zhijian
  1 sibling, 1 reply; 4+ messages in thread
From: Andrew Morton @ 2018-11-26 23:34 UTC (permalink / raw)
  To: Li Zhijian; +Cc: linux-kernel, Dominik Brodowski, zhijianx.li, Philip Li

On Fri, 16 Nov 2018 15:12:48 +0800 Li Zhijian <lizhijian@cn.fujitsu.com> wrote:

> Previously, sys_link() will fail due to the new path is already existed.
> this case ofen appears when we use a concated initrd, below is an
> sample:
> 
> 1) prepare a basic rootfs, it contains a regular files rc.local
> lizhijian@:~/yocto-tiny-i386-2016-04-22$ cat etc/rc.local
>  #!/bin/sh
>  echo "Running /etc/rc.local..."
> yocto-tiny-i386-2016-04-22$ find . | sed 's,^\./,,' | cpio -o -H newc | gzip -n -9 >../rootfs.cgz
> 
> 2) create a extra initrd which also includes a etc/rc.local
> lizhijian@:~/lkp-x86_64/etc$ echo "append initrd" >rc.local
> lizhijian@:~/lkp/lkp-x86_64/etc$ cat rc.local
> append initrd
> lizhijian@:~/lkp/lkp-x86_64/etc$ ln rc.local rc.local.hardlink
> append initrd
> lizhijian@:~/lkp/lkp-x86_64/etc$ stat rc.local rc.local.hardlink
>   File: 'rc.local'
>   Size: 14        	Blocks: 8          IO Block: 4096   regular file
> Device: 801h/2049d	Inode: 11296086    Links: 2
> Access: (0664/-rw-rw-r--)  Uid: ( 1002/lizhijian)   Gid: ( 1002/lizhijian)
> Access: 2018-11-15 16:08:28.654464815 +0800
> Modify: 2018-11-15 16:07:57.514903210 +0800
> Change: 2018-11-15 16:08:24.180228872 +0800
>  Birth: -
>   File: 'rc.local.hardlink'
>   Size: 14        	Blocks: 8          IO Block: 4096   regular file
> Device: 801h/2049d	Inode: 11296086    Links: 2
> Access: (0664/-rw-rw-r--)  Uid: ( 1002/lizhijian)   Gid: ( 1002/lizhijian)
> Access: 2018-11-15 16:08:28.654464815 +0800
> Modify: 2018-11-15 16:07:57.514903210 +0800
> Change: 2018-11-15 16:08:24.180228872 +0800
>  Birth: -
> 
> lizhijian@:~/lkp/lkp-x86_64$ find . | sed 's,^\./,,' | cpio -o -H newc | gzip -n -9 >../rc-local.cgz
> lizhijian@:~/lkp/lkp-x86_64$ gzip -dc ../rc-local.cgz | cpio -t
> .
> etc
> etc/rc.local.hardlink <<< it will be extracted first at this initrd
> etc/rc.local
> 
> 3) concate 2 initrds and boot
> lizhijian@:~/lkp$ cat rootfs.cgz rc-local.cgz >concate-initrd.cgz
> lizhijian@:~/lkp$ qemu-system-x86_64 -nographic -enable-kvm -cpu host -smp 1 -m 1024 -kernel ~/lkp/linux/arch/x86/boot/bzImage -append "console=ttyS0 earlyprint=ttyS0 ignore_loglevel" -initrd ./concate-initr.cgz -serial stdio -nodefaults
> 
> In this case, sys_link(2) will fail and return -EEXIST, so we can only
> get the rc.local at rootfs.cgz instead of rc-local.cgz

I can't claim to understand this, but I believe you ;)

How serious is this problem?  Do you think a 4.20 merge is justified? 
Or a -stable backport?  If so, why?

> --- a/init/initramfs.c
> +++ b/init/initramfs.c
> @@ -291,12 +291,16 @@ static int __init do_reset(void)
>  	return 1;
>  }
>  
> +static void __init clean_path(char *path, umode_t fmode);
> +
>  static int __init maybe_link(void)
>  {
>  	if (nlink >= 2) {
>  		char *old = find_link(major, minor, ino, mode, collected);
> -		if (old)
> +		if (old) {
> +			clean_path(collected, 0);
>  			return (ksys_link(old, collected) < 0) ? -1 : 1;
> +		}
>  	}
>  	return 0;
>  }

The forward declaration is unpleasing.  Why not simply move the function?

--- a/init/initramfs.c~initramfs-clean-old-path-before-creating-a-hardlink-fix
+++ a/init/initramfs.c
@@ -291,7 +291,17 @@ static int __init do_reset(void)
 	return 1;
 }
 
-static void __init clean_path(char *path, umode_t fmode);
+static void __init clean_path(char *path, umode_t fmode)
+{
+	struct kstat st;
+
+	if (!vfs_lstat(path, &st) && (st.mode ^ fmode) & S_IFMT) {
+		if (S_ISDIR(st.mode))
+			ksys_rmdir(path);
+		else
+			ksys_unlink(path);
+	}
+}
 
 static int __init maybe_link(void)
 {
@@ -305,18 +315,6 @@ static int __init maybe_link(void)
 	return 0;
 }
 
-static void __init clean_path(char *path, umode_t fmode)
-{
-	struct kstat st;
-
-	if (!vfs_lstat(path, &st) && (st.mode ^ fmode) & S_IFMT) {
-		if (S_ISDIR(st.mode))
-			ksys_rmdir(path);
-		else
-			ksys_unlink(path);
-	}
-}
-
 static __initdata int wfd;
 
 static int __init do_name(void)
_


^ permalink raw reply	[flat|nested] 4+ messages in thread
* Re: [PATCH] initramfs: clean old path before creating a hardlink
  2018-11-26 23:34 ` Andrew Morton
@ 2018-11-27  4:04   ` Li Zhijian
  0 siblings, 0 replies; 4+ messages in thread
From: Li Zhijian @ 2018-11-27  4:04 UTC (permalink / raw)
  To: Andrew Morton; +Cc: linux-kernel, Dominik Brodowski, zhijianx.li, Philip Li


On 11/27/2018 07:34 AM, Andrew Morton wrote:
> On Fri, 16 Nov 2018 15:12:48 +0800 Li Zhijian <lizhijian@cn.fujitsu.com> wrote:
>
>> Previously, sys_link() will fail due to the new path is already existed.
>> this case ofen appears when we use a concated initrd, below is an
>> sample:
>>
>> 1) prepare a basic rootfs, it contains a regular files rc.local
>> lizhijian@:~/yocto-tiny-i386-2016-04-22$ cat etc/rc.local
>>   #!/bin/sh
>>   echo "Running /etc/rc.local..."
>> yocto-tiny-i386-2016-04-22$ find . | sed 's,^\./,,' | cpio -o -H newc | gzip -n -9 >../rootfs.cgz
>>
>> 2) create a extra initrd which also includes a etc/rc.local
>> lizhijian@:~/lkp-x86_64/etc$ echo "append initrd" >rc.local
>> lizhijian@:~/lkp/lkp-x86_64/etc$ cat rc.local
>> append initrd
>> lizhijian@:~/lkp/lkp-x86_64/etc$ ln rc.local rc.local.hardlink
>> append initrd
>> lizhijian@:~/lkp/lkp-x86_64/etc$ stat rc.local rc.local.hardlink
>>    File: 'rc.local'
>>    Size: 14        	Blocks: 8          IO Block: 4096   regular file
>> Device: 801h/2049d	Inode: 11296086    Links: 2
>> Access: (0664/-rw-rw-r--)  Uid: ( 1002/lizhijian)   Gid: ( 1002/lizhijian)
>> Access: 2018-11-15 16:08:28.654464815 +0800
>> Modify: 2018-11-15 16:07:57.514903210 +0800
>> Change: 2018-11-15 16:08:24.180228872 +0800
>>   Birth: -
>>    File: 'rc.local.hardlink'
>>    Size: 14        	Blocks: 8          IO Block: 4096   regular file
>> Device: 801h/2049d	Inode: 11296086    Links: 2
>> Access: (0664/-rw-rw-r--)  Uid: ( 1002/lizhijian)   Gid: ( 1002/lizhijian)
>> Access: 2018-11-15 16:08:28.654464815 +0800
>> Modify: 2018-11-15 16:07:57.514903210 +0800
>> Change: 2018-11-15 16:08:24.180228872 +0800
>>   Birth: -
>>
>> lizhijian@:~/lkp/lkp-x86_64$ find . | sed 's,^\./,,' | cpio -o -H newc | gzip -n -9 >../rc-local.cgz
>> lizhijian@:~/lkp/lkp-x86_64$ gzip -dc ../rc-local.cgz | cpio -t
>> .
>> etc
>> etc/rc.local.hardlink <<< it will be extracted first at this initrd
>> etc/rc.local
>>
>> 3) concate 2 initrds and boot
>> lizhijian@:~/lkp$ cat rootfs.cgz rc-local.cgz >concate-initrd.cgz
>> lizhijian@:~/lkp$ qemu-system-x86_64 -nographic -enable-kvm -cpu host -smp 1 -m 1024 -kernel ~/lkp/linux/arch/x86/boot/bzImage -append "console=ttyS0 earlyprint=ttyS0 ignore_loglevel" -initrd ./concate-initr.cgz -serial stdio -nodefaults
>>
>> In this case, sys_link(2) will fail and return -EEXIST, so we can only
>> get the rc.local at rootfs.cgz instead of rc-local.cgz
> I can't claim to understand this, but I believe you ;)

thank you.

IMO, people who use concatenated initrd(cat rootfs.cgz rc-local.cgz >concatenated-initrd.cgz),
could expect that the later initrd(rc-local.cgz) can overwrite the previous initrd(rootfs.cgz).

However the previous kernel can not ensure this.

>
> How serious is this problem?  Do you think a 4.20 merge is justified?

It's critical for our scenario where it always concatenates multiple initrds.
and our scenario often use the latest kernel, so 4.20+ is good to us.



> Or a -stable backport?  If so, why?
>
> The forward declaration is unpleasing.  Why not simply move the function?

Got it, i will update in V2


Thanks

>
> --- a/init/initramfs.c~initramfs-clean-old-path-before-creating-a-hardlink-fix
> +++ a/init/initramfs.c
> @@ -291,7 +291,17 @@ static int __init do_reset(void)
>   	return 1;
>   }
>   
> -static void __init clean_path(char *path, umode_t fmode);
> +static void __init clean_path(char *path, umode_t fmode)
> +{
> +	struct kstat st;
> +
> +	if (!vfs_lstat(path, &st) && (st.mode ^ fmode) & S_IFMT) {
> +		if (S_ISDIR(st.mode))
> +			ksys_rmdir(path);
> +		else
> +			ksys_unlink(path);
> +	}
> +}
>   
>   static int __init maybe_link(void)
>   {
> @@ -305,18 +315,6 @@ static int __init maybe_link(void)
>   	return 0;
>   }
>   
> -static void __init clean_path(char *path, umode_t fmode)
> -{
> -	struct kstat st;
> -
> -	if (!vfs_lstat(path, &st) && (st.mode ^ fmode) & S_IFMT) {
> -		if (S_ISDIR(st.mode))
> -			ksys_rmdir(path);
> -		else
> -			ksys_unlink(path);
> -	}
> -}
> -
>   static __initdata int wfd;
>   
>   static int __init do_name(void)
> _
>
>
>
> .
>



^ permalink raw reply	[flat|nested] 4+ messages in thread
end of thread, other threads:[~2018-11-27  4:04 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-11-16  7:12 [PATCH] initramfs: clean old path before creating a hardlink Li Zhijian
2018-11-22  1:55 ` Li Zhijian
2018-11-26 23:34 ` Andrew Morton
2018-11-27  4:04   ` Li Zhijian

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.