!212 runc：delete do not ignore error from destroy

From: @zhong-jiawei-1 
Reviewed-by: @zhangsong234 
Signed-off-by: @zhangsong234

git-commit

@@ -1 +1 @@
-a10a8417e5eb298bf8cb83b3eb41bd1759dccde6
+363b21566639cffe19f04d29381cde0c6b272f8e

patch/0032-runc-delete-do-not-ignore-error-from-destroy.patch

@@ -0,0 +1,116 @@
+From 489e5bfbed5faff99d1fa48c146bd5a4f17b9c67 Mon Sep 17 00:00:00 2001
+From: Kir Kolyshkin <kolyshkin@gmail.com>
+Date: Mon, 6 Nov 2023 15:38:11 -0800
+Subject: [PATCH] runc delete: do not ignore error from destroy
+
+If container.Destroy() has failed, runc destroy still return 0, which is
+wrong and can result in other issues down the line.
+
+Let's always return error from destroy in runc delete.
+
+For runc checkpoint and runc run, we still treat it as a warning.
+
+Co-authored-by: Zhang Tianyang <burning9699@gmail.com>
+Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
+---
+ checkpoint.go                   |  6 +++++-
+ delete.go                       |  7 ++-----
+ libcontainer/container_linux.go |  5 ++++-
+ utils_linux.go                  | 10 +++-------
+ 4 files changed, 14 insertions(+), 14 deletions(-)
+
+diff --git a/checkpoint.go b/checkpoint.go
+index 32a62a8b..897564c1 100644
+--- a/checkpoint.go
++++ b/checkpoint.go
+@@ -65,7 +65,11 @@ checkpointed.`,
+ 		options := criuOptions(context)
+ 		if !(options.LeaveRunning || options.PreDump) {
+ 			// destroy container unless we tell CRIU to keep it
+-			defer destroy(container)
++			defer func() {
++				if err := container.Destroy(); err != nil {
++                                	logrus.Warn(err)
++                        	}
++			}()
+ 		}
+ 		// these are the mandatory criu options for a container
+ 		setPageServer(context, options)
+diff --git a/delete.go b/delete.go
+index 799c2a77..6fe776d0 100644
+--- a/delete.go
++++ b/delete.go
+@@ -18,8 +18,7 @@ func killContainer(container libcontainer.Container) error {
+ 	for i := 0; i < 10; i++ {
+ 		time.Sleep(100 * time.Millisecond)
+ 		if err := container.Signal(unix.Signal(0), false); err != nil {
+-			destroy(container)
+-			return nil
++			return container.Destroy()
+ 		}
+ 	}
+ 	return errors.New("container init still running")
+@@ -72,7 +71,7 @@ status of "ubuntu01" as "stopped" the following will delete resources held for
+ 		}
+ 		switch s {
+ 		case libcontainer.Stopped:
+-			destroy(container)
++			return container.Destroy()
+ 		case libcontainer.Created:
+ 			return killContainer(container)
+ 		default:
+@@ -81,7 +80,5 @@ status of "ubuntu01" as "stopped" the following will delete resources held for
+ 			}
+ 			return fmt.Errorf("cannot delete container %s that is not stopped: %s", id, s)
+ 		}
+-
+-		return nil
+ 	},
+ }
+diff --git a/libcontainer/container_linux.go b/libcontainer/container_linux.go
+index f56d73e6..4f9433b8 100644
+--- a/libcontainer/container_linux.go
++++ b/libcontainer/container_linux.go
+@@ -708,7 +708,10 @@ func (c *linuxContainer) newInitConfig(process *Process) *initConfig {
+ func (c *linuxContainer) Destroy() error {
+ 	c.m.Lock()
+ 	defer c.m.Unlock()
+-	return c.state.destroy()
++	if err := c.state.destroy(); err != nil {
++		return fmt.Errorf("unable to destroy container: %w", err)
++	}
++	return nil
+ }
+ 
+ func (c *linuxContainer) Pause() error {
+diff --git a/utils_linux.go b/utils_linux.go
+index a9badf20..20a949a1 100644
+--- a/utils_linux.go
++++ b/utils_linux.go
+@@ -114,12 +114,6 @@ func newProcess(p specs.Process) (*libcontainer.Process, error) {
+ 	return lp, nil
+ }
+ 
+-func destroy(container libcontainer.Container) {
+-	if err := container.Destroy(); err != nil {
+-		logrus.Error(err)
+-	}
+-}
+-
+ // setupIO modifies the given process config according to the options.
+ func setupIO(process *libcontainer.Process, rootuid, rootgid int, createTTY, detach bool, sockpath string) (*tty, error) {
+ 	if createTTY {
+@@ -323,7 +317,9 @@ func (r *runner) run(config *specs.Process) (int, error) {
+ 
+ func (r *runner) destroy() {
+ 	if r.shouldDestroy {
+-		destroy(r.container)
++		if err := r.container.Destroy(); err != nil {
++			logrus.Warn(err)
++		}
+ 	}
+ }
+ 
+-- 
+2.33.0
+

runc.spec

@@ -3,7 +3,7 @@
 
 Name: runc
 Version: 1.1.8
-Release: 9
+Release: 10
 Summary: runc is a CLI tool for spawning and running containers according to the OCI specification.
 
 License: ASL 2.0
@@ -54,6 +54,12 @@ install -p -m 755 runc $RPM_BUILD_ROOT/%{_bindir}/runc
 %{_bindir}/runc
 
 %changelog
+* Mon Dec 4 2023 zhongjiawei<zhongjiawei1@huawei.com> - 1.1.8-10
+- Type:bugfix
+- CVE:NA
+- SUG:NA
+- DESC:runc delete do not ignore error from destroy
+
 * Thu Nov 16 2023 zhongjiawei<zhongjiawei1@huawei.com> - 1.1.8-9
 - Type:bugfix
 - CVE:NA

series.conf

@@ -28,3 +28,4 @@ patch/0028-runc-nsenter-cloned_binary-remove-bindfd-logic-entirely.patch
 patch/0029-runc-libct-init-unify-init-fix-its-error-logic.patch
 patch/0030-runc-Handle-kmem.limit_in_bytes-removal.patch
 patch/0031-runc-fix-update-rt-runtime-us-and-rt-period-us-faile.patch
+patch/0032-runc-delete-do-not-ignore-error-from-destroy.patch