mbox series

[SRU,J/K/J:hwe-5.17[PATCH,0/2] refactoring of overlayfs fix to properly support shiftfs

Message ID 20220805063214.32432-1-andrea.righi@canonical.com
Headers show
Series refactoring of overlayfs fix to properly support shiftfs | expand

Message

Andrea Righi Aug. 5, 2022, 6:32 a.m. UTC
[Impact]

Starting with 5.13 we've incorrectly dropped the following sauce patch:

  UBUNTU: SAUCE: overlayfs: fix incorrect mnt_id of files opened from map_files

This patch is required to use overlayfs on top of shiftfs and without
this patch we may break containers that rely on shiftfs (using zfs/ceph
as storage pool w/ shiftfs enabled).

However, we made this patch dependent on AUFS, starting with Jammy we're
not enabling AUFS anymore, so this fix becomes a no-op.

So we need to re-introduce this fix with a bit of refactoring to not
depend on AUFS.

[Test case]

The following script can be used to trigger the issue:

  #!/bin/bash

  cat > test.py << EOF
  import sys

  f = open("/proc/self/maps")

  for l in f.readlines():
    if "python" not in l:
      continue
    print(l)
    s = l.split()
    start, end = s[0].split("-")
    fname = s[-1]
    print(start, end, fname)
    break
  else:
    sys.exit(1)

  test_file1 = open(fname)
  test_file2 = open("/proc/self/map_files/%s-%s" % (start, end))

  fdinfo1 = open("/proc/self/fdinfo/%d" % test_file1.fileno()).read()
  fdinfo2 = open("/proc/self/fdinfo/%d" % test_file2.fileno()).read()

  if fdinfo1 != fdinfo2:
    print("FAIL")
    print(test_file1)
    print(fdinfo1)
    print(test_file2)
    print(fdinfo2)
    sys.exit(1)
  print("PASS")
  EOF
  sudo docker run -it --privileged --rm -v `pwd`:/mnt python python /mnt/test.py

[Fix]

Import the right pieces from AUFS to properly support the fix and get
rid of the AUFS dependency across all our kernels and re-apply the
overlayfs fix without the AUFS dependency.

[Regression potential]

This patch is touching overlayfs, so we may see potential regressions in
overlayfs, especially when containers are used.

[Additional notes]

We need different patches for Jammy and Kinetic, because Kinetic is
missing AUFS completely, so we need to import the chunk of code from
AUFS that we were relying to, in order to properly apply the overlayfs
fix.

Moreover, J/hwe-5.17 also needs a fix, because this kernel is now
"detached" (it doesn't have a parent anymore), so it's following its own
development workflow, like a master kernel.

Comments

Stefan Bader Aug. 5, 2022, 7:50 a.m. UTC | #1
On 05.08.22 08:32, Andrea Righi wrote:
> [Impact]
> 
> Starting with 5.13 we've incorrectly dropped the following sauce patch:
> 
>    UBUNTU: SAUCE: overlayfs: fix incorrect mnt_id of files opened from map_files
> 
> This patch is required to use overlayfs on top of shiftfs and without
> this patch we may break containers that rely on shiftfs (using zfs/ceph
> as storage pool w/ shiftfs enabled).
> 
> However, we made this patch dependent on AUFS, starting with Jammy we're
> not enabling AUFS anymore, so this fix becomes a no-op.
> 
> So we need to re-introduce this fix with a bit of refactoring to not
> depend on AUFS.
> 
> [Test case]
> 
> The following script can be used to trigger the issue:
> 
>    #!/bin/bash
> 
>    cat > test.py << EOF
>    import sys
> 
>    f = open("/proc/self/maps")
> 
>    for l in f.readlines():
>      if "python" not in l:
>        continue
>      print(l)
>      s = l.split()
>      start, end = s[0].split("-")
>      fname = s[-1]
>      print(start, end, fname)
>      break
>    else:
>      sys.exit(1)
> 
>    test_file1 = open(fname)
>    test_file2 = open("/proc/self/map_files/%s-%s" % (start, end))
> 
>    fdinfo1 = open("/proc/self/fdinfo/%d" % test_file1.fileno()).read()
>    fdinfo2 = open("/proc/self/fdinfo/%d" % test_file2.fileno()).read()
> 
>    if fdinfo1 != fdinfo2:
>      print("FAIL")
>      print(test_file1)
>      print(fdinfo1)
>      print(test_file2)
>      print(fdinfo2)
>      sys.exit(1)
>    print("PASS")
>    EOF
>    sudo docker run -it --privileged --rm -v `pwd`:/mnt python python /mnt/test.py
> 
> [Fix]
> 
> Import the right pieces from AUFS to properly support the fix and get
> rid of the AUFS dependency across all our kernels and re-apply the
> overlayfs fix without the AUFS dependency.
> 
> [Regression potential]
> 
> This patch is touching overlayfs, so we may see potential regressions in
> overlayfs, especially when containers are used.
> 
> [Additional notes]
> 
> We need different patches for Jammy and Kinetic, because Kinetic is
> missing AUFS completely, so we need to import the chunk of code from
> AUFS that we were relying to, in order to properly apply the overlayfs
> fix.
> 
> Moreover, J/hwe-5.17 also needs a fix, because this kernel is now
> "detached" (it doesn't have a parent anymore), so it's following its own
> development workflow, like a master kernel.
> 
> 
> 
Acked-by: Stefan Bader <stefan.bader@canonical.com>
Kleber Souza Aug. 16, 2022, 4:21 p.m. UTC | #2
On 05.08.22 08:32, Andrea Righi wrote:
> [Impact]
> 
> Starting with 5.13 we've incorrectly dropped the following sauce patch:
> 
>    UBUNTU: SAUCE: overlayfs: fix incorrect mnt_id of files opened from map_files
> 
> This patch is required to use overlayfs on top of shiftfs and without
> this patch we may break containers that rely on shiftfs (using zfs/ceph
> as storage pool w/ shiftfs enabled).
> 
> However, we made this patch dependent on AUFS, starting with Jammy we're
> not enabling AUFS anymore, so this fix becomes a no-op.
> 
> So we need to re-introduce this fix with a bit of refactoring to not
> depend on AUFS.
> 
> [Test case]
> 
> The following script can be used to trigger the issue:
> 
>    #!/bin/bash
> 
>    cat > test.py << EOF
>    import sys
> 
>    f = open("/proc/self/maps")
> 
>    for l in f.readlines():
>      if "python" not in l:
>        continue
>      print(l)
>      s = l.split()
>      start, end = s[0].split("-")
>      fname = s[-1]
>      print(start, end, fname)
>      break
>    else:
>      sys.exit(1)
> 
>    test_file1 = open(fname)
>    test_file2 = open("/proc/self/map_files/%s-%s" % (start, end))
> 
>    fdinfo1 = open("/proc/self/fdinfo/%d" % test_file1.fileno()).read()
>    fdinfo2 = open("/proc/self/fdinfo/%d" % test_file2.fileno()).read()
> 
>    if fdinfo1 != fdinfo2:
>      print("FAIL")
>      print(test_file1)
>      print(fdinfo1)
>      print(test_file2)
>      print(fdinfo2)
>      sys.exit(1)
>    print("PASS")
>    EOF
>    sudo docker run -it --privileged --rm -v `pwd`:/mnt python python /mnt/test.py
> 
> [Fix]
> 
> Import the right pieces from AUFS to properly support the fix and get
> rid of the AUFS dependency across all our kernels and re-apply the
> overlayfs fix without the AUFS dependency.
> 
> [Regression potential]
> 
> This patch is touching overlayfs, so we may see potential regressions in
> overlayfs, especially when containers are used.
> 
> [Additional notes]
> 
> We need different patches for Jammy and Kinetic, because Kinetic is
> missing AUFS completely, so we need to import the chunk of code from
> AUFS that we were relying to, in order to properly apply the overlayfs
> fix.
> 
> Moreover, J/hwe-5.17 also needs a fix, because this kernel is now
> "detached" (it doesn't have a parent anymore), so it's following its own
> development workflow, like a master kernel.
> 
> 
> 


Acked-by: Kleber Sacilotto de Souza <kleber.souza@canonical.com>

Thanks
Andrea Righi Aug. 17, 2022, 6:29 a.m. UTC | #3
On Fri, Aug 05, 2022 at 08:32:09AM +0200, Andrea Righi wrote:
> [Impact]
> 
> Starting with 5.13 we've incorrectly dropped the following sauce patch:
> 
>   UBUNTU: SAUCE: overlayfs: fix incorrect mnt_id of files opened from map_files
> 
> This patch is required to use overlayfs on top of shiftfs and without
> this patch we may break containers that rely on shiftfs (using zfs/ceph
> as storage pool w/ shiftfs enabled).
> 
> However, we made this patch dependent on AUFS, starting with Jammy we're
> not enabling AUFS anymore, so this fix becomes a no-op.
> 
> So we need to re-introduce this fix with a bit of refactoring to not
> depend on AUFS.
> 
> [Test case]
> 
> The following script can be used to trigger the issue:
> 
>   #!/bin/bash
> 
>   cat > test.py << EOF
>   import sys
> 
>   f = open("/proc/self/maps")
> 
>   for l in f.readlines():
>     if "python" not in l:
>       continue
>     print(l)
>     s = l.split()
>     start, end = s[0].split("-")
>     fname = s[-1]
>     print(start, end, fname)
>     break
>   else:
>     sys.exit(1)
> 
>   test_file1 = open(fname)
>   test_file2 = open("/proc/self/map_files/%s-%s" % (start, end))
> 
>   fdinfo1 = open("/proc/self/fdinfo/%d" % test_file1.fileno()).read()
>   fdinfo2 = open("/proc/self/fdinfo/%d" % test_file2.fileno()).read()
> 
>   if fdinfo1 != fdinfo2:
>     print("FAIL")
>     print(test_file1)
>     print(fdinfo1)
>     print(test_file2)
>     print(fdinfo2)
>     sys.exit(1)
>   print("PASS")
>   EOF
>   sudo docker run -it --privileged --rm -v `pwd`:/mnt python python /mnt/test.py
> 
> [Fix]
> 
> Import the right pieces from AUFS to properly support the fix and get
> rid of the AUFS dependency across all our kernels and re-apply the
> overlayfs fix without the AUFS dependency.
> 
> [Regression potential]
> 
> This patch is touching overlayfs, so we may see potential regressions in
> overlayfs, especially when containers are used.
> 
> [Additional notes]
> 
> We need different patches for Jammy and Kinetic, because Kinetic is
> missing AUFS completely, so we need to import the chunk of code from
> AUFS that we were relying to, in order to properly apply the overlayfs
> fix.
> 
> Moreover, J/hwe-5.17 also needs a fix, because this kernel is now
> "detached" (it doesn't have a parent anymore), so it's following its own
> development workflow, like a master kernel.

Applied to kinetic/linux.

-Andrea
Kleber Souza Aug. 17, 2022, 9:06 a.m. UTC | #4
On 05.08.22 08:32, Andrea Righi wrote:
> [Impact]
> 
> Starting with 5.13 we've incorrectly dropped the following sauce patch:
> 
>    UBUNTU: SAUCE: overlayfs: fix incorrect mnt_id of files opened from map_files
> 
> This patch is required to use overlayfs on top of shiftfs and without
> this patch we may break containers that rely on shiftfs (using zfs/ceph
> as storage pool w/ shiftfs enabled).
> 
> However, we made this patch dependent on AUFS, starting with Jammy we're
> not enabling AUFS anymore, so this fix becomes a no-op.
> 
> So we need to re-introduce this fix with a bit of refactoring to not
> depend on AUFS.
> 
> [Test case]
> 
> The following script can be used to trigger the issue:
> 
>    #!/bin/bash
> 
>    cat > test.py << EOF
>    import sys
> 
>    f = open("/proc/self/maps")
> 
>    for l in f.readlines():
>      if "python" not in l:
>        continue
>      print(l)
>      s = l.split()
>      start, end = s[0].split("-")
>      fname = s[-1]
>      print(start, end, fname)
>      break
>    else:
>      sys.exit(1)
> 
>    test_file1 = open(fname)
>    test_file2 = open("/proc/self/map_files/%s-%s" % (start, end))
> 
>    fdinfo1 = open("/proc/self/fdinfo/%d" % test_file1.fileno()).read()
>    fdinfo2 = open("/proc/self/fdinfo/%d" % test_file2.fileno()).read()
> 
>    if fdinfo1 != fdinfo2:
>      print("FAIL")
>      print(test_file1)
>      print(fdinfo1)
>      print(test_file2)
>      print(fdinfo2)
>      sys.exit(1)
>    print("PASS")
>    EOF
>    sudo docker run -it --privileged --rm -v `pwd`:/mnt python python /mnt/test.py
> 
> [Fix]
> 
> Import the right pieces from AUFS to properly support the fix and get
> rid of the AUFS dependency across all our kernels and re-apply the
> overlayfs fix without the AUFS dependency.
> 
> [Regression potential]
> 
> This patch is touching overlayfs, so we may see potential regressions in
> overlayfs, especially when containers are used.
> 
> [Additional notes]
> 
> We need different patches for Jammy and Kinetic, because Kinetic is
> missing AUFS completely, so we need to import the chunk of code from
> AUFS that we were relying to, in order to properly apply the overlayfs
> fix.
> 
> Moreover, J/hwe-5.17 also needs a fix, because this kernel is now
> "detached" (it doesn't have a parent anymore), so it's following its own
> development workflow, like a master kernel.
> 
> 
> 

Applied to jammy:linux-hwe-5.17.

Thanks,
Kleber