From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mail-wr1-f48.google.com (mail-wr1-f48.google.com [209.85.221.48]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id C341F4F5E0 for ; Wed, 1 Apr 2026 09:11:45 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=209.85.221.48 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1775034708; cv=none; b=EzwdYxQPjiZddWGRkFba2OrmrnyIw3pC31TD2IZARhRx77oVnXHYakJ+qTbDxoyUGvLexjvv+TwQSiXN/vcR+b605t1/jAetkOxrde6HLqoXf2cN+dJDSMLX43chQ2jqKjWakKB77mgDK9F3td4hm649oHyNZdbAMYt7nQudR4k= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1775034708; c=relaxed/simple; bh=//Ff137ItcAe5Ga2LCorRIy8/izw4y7+oMbb8vVEi8c=; h=Date:From:To:Cc:Subject:Message-ID:References:MIME-Version: Content-Type:Content-Disposition:In-Reply-To; b=DY4df9sdm5gpFN0JTPIpTH3WACvNz3QIN+3GHqxbhX/IKaxNaFljSjFgWNkkKBZX2Ryn0REz4RLCchFM3XyRg6+V+1ipe0IioEHcT26RR61XGwiU/+cBAfKgdql5sVwukNAd+g29OcjtViwil1BoEqkehjzTpui8bbJTIG+657A= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=gmail.com; spf=pass smtp.mailfrom=gmail.com; dkim=pass (2048-bit key) header.d=gmail.com header.i=@gmail.com header.b=YW7kgTJm; arc=none smtp.client-ip=209.85.221.48 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=gmail.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=gmail.com Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=gmail.com header.i=@gmail.com header.b="YW7kgTJm" Received: by mail-wr1-f48.google.com with SMTP id ffacd0b85a97d-43cfb723793so2050502f8f.2 for ; Wed, 01 Apr 2026 02:11:45 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20251104; t=1775034704; x=1775639504; darn=vger.kernel.org; h=in-reply-to:content-disposition:mime-version:references:message-id :subject:cc:to:from:date:from:to:cc:subject:date:message-id:reply-to; bh=GU2MYtgtof8zhGg7Ukz6W6ZgS/Q4jFeOlOLWhznUGu0=; b=YW7kgTJmmw6y/ie63CWy9Hr1fkceIYROLSpjR7VHWWqzOoDxoKX+sMW32vHtaS6tq4 rWnAcpiCJho3Wx7jbwXxMdvUHtToYXK2Ujg5ML2SiF7o6tXQvaY9gLJGwMiDdAGaUBNa 3M2cyM0LixwO+Ceb5LTq9iyd+C7rGUtqqcgj99lCEqRSQZIWX2HpENnC5wIfKx/Ur5Yc LiW0nz7da+UKPaVAzgLr8TDSIVTGPK/rQ/BMS4QmdTGbwXq5KQlVXW4TFCHEhXjXPeXk 5CQVqiRFxEWwKYE7BcFnB8pNDF57dMeOfFeeIsG6WU6WY7WBCJd/jV5SfIvWgAW5FmPD bXXA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20251104; t=1775034704; x=1775639504; h=in-reply-to:content-disposition:mime-version:references:message-id :subject:cc:to:from:date:x-gm-gg:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=GU2MYtgtof8zhGg7Ukz6W6ZgS/Q4jFeOlOLWhznUGu0=; b=BDhKitYOJ8AKkoeCfuTiZ+7jVEKNW/gLF7JpIB8Eud5nmKQRhDoyDj7DJlv5CM+2Qw y4FvDzNLg34jigMS5R2ctt9gCBjayB4zEzPovKbZuhEb7X78rerW2X4GBDveVPfhPvBx QyC3EmYDKYjMEabJyjdLAQUN15BdnxWq8zWN9cE3Wenlj/GvaxUnHg6EzvMe3YI9z/b6 vH+L+fLunIp48Nn/RJHP477HYI5zbRckC3EnMqTojOLO3Ry6W306Xlahj2hlPtn/mLTv LgTb44alhsln2iOL/Zp0qcHS4Cmjb/4NwuCjSi1UCPEsbcDbsBqQuh1ZVtogGhAMKv8K EEog== X-Forwarded-Encrypted: i=1; AJvYcCVlHRqKO+axdfTSQK12YqgYQxZbUD0+vdRUvUZdddHjaMSPIj+Jj/qnvx4alibSBJxwyk2ZRLZHac9y6bM=@vger.kernel.org X-Gm-Message-State: AOJu0Yxuz3X97U9AODXKBmm+Dzj8/bdBJHx8aH2chov/eGYl+iLEHjcu QYijpTSAHpN1+PUrF2qbjFU3kxgtDjNaD3sOTDcV+bZXM9YmNTkdYNHJdbsQ9w== X-Gm-Gg: ATEYQzyUJBAv+VfRJjwmGJMGAzQ5ClKWh/58Ui2FqxBa+s0XeSvD78ZtGP2BjPhTHhZ QkwYPcg/QRJqAKT4fVYt5XSE3jWM5jKrh3UoVAfyEqqRy9yec/p147XaryWxTYhi8VQKXaZ6V1J fMLIMisHtL8GAZqhGTFNQqvIZGKZVFFB7SvTI711wlCts0L4ohLpVw6QNg3I8GUutlIEhdzoDyd Uu0ZvjaEeF4CwxRiIp/dR8/u7NPEBpyNebNJS5L6dyDC/OsoubFfvypHnIHSTo351FOYS0PXk0Q VU9M0F/nXsMLGKzVxW/JfdyyySQ/NQ0R+SV/vvQGC0RLvhbrbM3jepg3WwN2xB1UKv2DYVrXfeF tXZ5gcczoV/bTVW49FA0jxjR9dMG+ZR8Teyoan6+EhPdZ9Uhy/ZnbxCTH+rR1Bzb7wQLBrngtAl oczeKwtzKeE58buw98hIE= X-Received: by 2002:a05:600c:8184:b0:485:9a50:3370 with SMTP id 5b1f17b1804b1-48883575bbamr42898405e9.8.1775034703589; Wed, 01 Apr 2026 02:11:43 -0700 (PDT) Received: from localhost ([196.207.164.177]) by smtp.gmail.com with ESMTPSA id ffacd0b85a97d-43cf257cbc3sm33508339f8f.35.2026.04.01.02.11.42 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Wed, 01 Apr 2026 02:11:42 -0700 (PDT) Date: Wed, 1 Apr 2026 12:11:39 +0300 From: Dan Carpenter To: Yuho Choi Cc: Andy Shevchenko , Hans de Goede , Mauro Carvalho Chehab , Sakari Ailus , Greg Kroah-Hartman , Peter Zijlstra , Kees Cook , Josh Poimboeuf , Thomas Andreatta , linux-media@vger.kernel.org, linux-staging@lists.linux.dev, linux-kernel@vger.kernel.org, Yuho Choi Subject: Re: [PATCH v2] media: atomisp: gc2235: fix UAF and memory leak Message-ID: References: <20260331194727.52054-1-yqc5929@psu.edu> Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20260331194727.52054-1-yqc5929@psu.edu> On Tue, Mar 31, 2026 at 03:47:27PM -0400, Yuho Choi wrote: > gc2235_probe() handles its error paths incorrectly. > > If media_entity_pads_init() fails, gc2235_remove() is called, which > tears down the subdev and frees dev, but then still falls through to > atomisp_register_i2c_module(). This results in use-after-free. > > If atomisp_register_i2c_module() fails, the media entity and control > handler are left initialized and dev is leaked. > > Handle each failure path locally and unwind only the initialized > resources. Return success only after the full probe sequence completes. > > Signed-off-by: Yuho Choi Needs a Fixes tag. The design of this code is that gc2235_remove() is supposed to clean up from every type of possible error. This style of error handling is always buggy: https://staticthinking.wordpress.com/2025/03/31/reviewing-magical-cleanup-functions/ However, the commit message does not explain why it's buggy in this case and there are still two error paths that use gc2235_remove() to clean up. Please, could you fix those two error paths and explain why we cannot just do: ret = atomisp_register_i2c_module(); if (ret) goto out_free; return 0; out_free: gc2235_remove(client); return ret; regards, dan carpenter