From:   Bernard Zhao <bernard@vivo.com>
To:     Paul Moore <paul@paul-moore.com>,
        Stephen Smalley <stephen.smalley.work@gmail.com>,
        Eric Paris <eparis@parisplace.org>, selinux@vger.kernel.org,
        linux-kernel@vger.kernel.org
Cc:     Bernard Zhao <bernard@vivo.com>
Subject: [PATCH v3] security/selinux: fix potential memleak in error branch
Date:   Fri, 10 Dec 2021 04:03:58 -0800
Message-Id: <20211210120359.394986-1-bernard@vivo.com>
Content-Transfer-Encoding: 8bit
Content-Type: text/plain
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?us-ascii?Q?EeBZLwkm9hzO49rEm1MkLuu0u6ymF0S+civGxjhzLGRdIcya9QmbZxYPBZlp?=
 =?us-ascii?Q?tmubZEAGZaVwwksXBgQyaXFGOyYyl372M/gJe47cjo/hTq3CsC62I80Fpx9x?=
 =?us-ascii?Q?hV+dmK4Vv8jQeO5Hg3arZcCYrnjaRlZjJnQ1yrqrlqjWUKJumWKqwKVvSk+X?=
 =?us-ascii?Q?B8NRduZy2Qh+LjvzdNy3GSUDEAqrgffQO4JjuReqChHWJLxk78H+YJh4tvnb?=
 =?us-ascii?Q?xkjE0exSXR0KnqNnYOhSSm6TPdA1so3mSEaWGGk3GuwfP+1caGaoJRFYnb7w?=
 =?us-ascii?Q?V9RpJXfy8KGFixiVNi5bVeltL19s1RcDXpLQOvBuU92CZTOIvj1hhHF7tWtL?=
 =?us-ascii?Q?r5QHlwdqAeZKp42Q7Z4QkipnfLzpZpgHImsGareJMhNJYpt+KdS+7Y8kKUQO?=
 =?us-ascii?Q?hvegacrrKDNvtGfTl/dnM52KR9glZVCQBzYl7MZnPgm+EcpuEwnr8qEIFtrF?=
 =?us-ascii?Q?9VE9KPBzLw388dAx07KIdafsktSpWfSGc0a/XgmcPIjF9fLEH079aQOPA4nb?=
 =?us-ascii?Q?n6LWPIPnQCNAW5jzW92kD6MFvg6fTsXLmNtrfPbBxTRqvsBdViZzeWqcB14D?=
 =?us-ascii?Q?XMoBRNBCKg9tPaHRBc9d0VFtpT5Chwymw3SOWQCIaAG6fa5m6wJdIIx9wBqi?=
 =?us-ascii?Q?/qJ1D92HuzyLic2BUuKAlasBgmwdjqp389ifHejBdYcStfqOwnU81g+Du4cI?=
 =?us-ascii?Q?93f98mBjk0miHlvgDrX7QJEXsOyvnw8bkqqRtVGMb3ihSyBfBq4cN8YhlYEe?=
 =?us-ascii?Q?qrUZcF+8JodsFJoRnUEjc+nTGwO+0H+6OPXFKI+8HLURxQ8dBiF+BikhiBMI?=
 =?us-ascii?Q?F1jkR6MaSp6QFK2q0Uy13Q3EGLVa0sZscqwM1GJ2FNWLm2yDE34A8u3Izzrw?=
 =?us-ascii?Q?rjxiFJZExbC2WUh7FFoTssLYl2IJ06c/HTa+0aX7rmzeeUKYLwvjx5KXSdJc?=
 =?us-ascii?Q?NqgVlvRwVszm6COqIkIwytaYwq9wESVu1r6fPDxCvt0zhxcnRpZqUbmK3ibC?=
 =?us-ascii?Q?GLkdxjXLpvae632I2m7lyrUhiXJDsuml5DlSY74L7FBNW0SQIgtzDy/y2xy2?=
 =?us-ascii?Q?U5eU2B8wYH8UJAPGiIxP8Jua9tbXCUU4zxII6tuf950KeACVZAl5QiJhFV+d?=
 =?us-ascii?Q?xr9f1lXxTEVZ8gsRsCDaiefsXkkOFw0YkPvSc2u7Sh+Ju4C5nSdbRoeaxGmG?=
 =?us-ascii?Q?z5X+fplJBdnEH+j6lm65m7+Q+6rsKEjzGE3VwYJfSqO1hvD/BDCCeOLwwXzb?=
 =?us-ascii?Q?sf88WCzkwgiFyXNuZDI1IkDmeXCpkK5+dimX4s561TtYFanIaKk7JJWTd62V?=
 =?us-ascii?Q?+Dg8aB2vXgkJfJAM9dolX3icsd+490vs9UqcOCAWI5ykVRhr90Los5bEv77c?=
 =?us-ascii?Q?+rUQJUxtHdn4QHkmG/GfU7l9NXmLR2Vqoa/Kb0EAgl8B8aITNHdqYo5T+Cnp?=
 =?us-ascii?Q?/Z2lFr/Ga39mfouH/KoLs73hqg3lide7fmudiiPAkkHUZ9e22lL7T8v1zTG3?=
 =?us-ascii?Q?g+zIP5bKNBIxy/vKGOmA+wYHnJ+P6iFB6oR/Xq7ZiqcsBq8whzr+V8mXRsKB?=
 =?us-ascii?Q?6/MvGI9nryru5RuLdtL1GCv2Uhv4p9OZcww1DPlcnlU7O7vPNlgw1S8BNnEZ?=
 =?us-ascii?Q?dUymTeNIf4aZaqY1qfDlnsk=3D?=
X-OriginatorOrg: vivo.com
X-MS-Exchange-CrossTenant-Network-Message-Id: 706668b9-8745-4e59-1eb8-08d9bbd52c72
X-MS-Exchange-CrossTenant-AuthSource: PSAPR06MB4021.apcprd06.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 10 Dec 2021 12:04:10.0083
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: 923e42dc-48d5-4cbe-b582-1a797a6412ed
X-MS-Exchange-CrossTenant-MailboxType: HOSTED
X-MS-Exchange-CrossTenant-UserPrincipalName: lX4uAkWAk4QYag0vf5MgWSNjeNVCFlsKmOXwSpNQNaGkgJNUwIQpNjB3kzvN6Ysot0hHAHCWbr03ZVf3n9AfXA==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: PSAPR06MB4326
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

This patch try to fix potential memleak in error branch.

Signed-off-by: Bernard Zhao <bernard@vivo.com>

Changes since V1:
*make it to be simpler to do the "(!s)" check before the "(!opts)" check.

Changes since v2:
*add *mnt_opts = NULL after kfree(opt) to avoid double free risk.
---
 security/selinux/hooks.c | 12 ++++++++++--
 1 file changed, 10 insertions(+), 2 deletions(-)

diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
index 62d30c0a30c2..0d018f054dfb 100644
--- a/security/selinux/hooks.c
+++ b/security/selinux/hooks.c
@@ -983,18 +983,22 @@ static int selinux_sb_clone_mnt_opts(const struct super_block *oldsb,
 static int selinux_add_opt(int token, const char *s, void **mnt_opts)
 {
 	struct selinux_mnt_opts *opts = *mnt_opts;
+	bool is_alloc_opts = false;
 
 	if (token == Opt_seclabel)	/* eaten and completely ignored */
 		return 0;
 
+	if (!s)
+		return -ENOMEM;
+
 	if (!opts) {
 		opts = kzalloc(sizeof(struct selinux_mnt_opts), GFP_KERNEL);
 		if (!opts)
 			return -ENOMEM;
 		*mnt_opts = opts;
+		is_alloc_opts = true;
 	}
-	if (!s)
-		return -ENOMEM;
+
 	switch (token) {
 	case Opt_context:
 		if (opts->context || opts->defcontext)
@@ -1019,6 +1023,10 @@ static int selinux_add_opt(int token, const char *s, void **mnt_opts)
 	}
 	return 0;
 Einval:
+	if (is_alloc_opts) {
+		kfree(opts);
+		*mnt_opts = NULL;
+	}
 	pr_warn(SEL_MOUNT_FAIL_MSG);
 	return -EINVAL;
 }
-- 
2.33.1