From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mail-wm1-f53.google.com (mail-wm1-f53.google.com [209.85.128.53]) (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 3788B1DD885 for ; Mon, 6 Jan 2025 11:35:42 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=209.85.128.53 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1736163345; cv=none; b=BdTrl7YCbLodYnEzKw35jrfOkEzPnBeXeRwrqqdFdiYEAwKZ6GxhbiYMdj4F6MX1DriyJBHmoDgne0SYLmjYY60fqChuDvMrs58GcobvBDt4/bT+aKimWOaD5oKyoqGhp90aZewhczzbn+smM/AyXX7OvhbW5IhtW5T8EFQunH8= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1736163345; c=relaxed/simple; bh=h9x7fU3zMZ3O9SaobNmcIu72VxHRH47G5BsxrJmlDJ4=; h=Date:From:To:Cc:Subject:Message-ID:References:MIME-Version: Content-Type:Content-Disposition:In-Reply-To; b=dTd0FMc9pJ52axYaPqf3Er2NMcXOYzJhBuQJeVbQ574ua6jjqdbhn5JtmMZwChnpSqEkl5EJxI7QgSlehwYr3nI6gZvVMcphKNeHWVFbkxqpcjT8BnQRiXDQzKsIXCPOTNQq/p8YTj+opraK3+sqNlWB43B/Y1dBKe5VAhXfK5g= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=linaro.org; spf=pass smtp.mailfrom=linaro.org; dkim=pass (2048-bit key) header.d=linaro.org header.i=@linaro.org header.b=yReKwwIl; arc=none smtp.client-ip=209.85.128.53 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=linaro.org Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=linaro.org Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=linaro.org header.i=@linaro.org header.b="yReKwwIl" Received: by mail-wm1-f53.google.com with SMTP id 5b1f17b1804b1-4361f664af5so162561165e9.1 for ; Mon, 06 Jan 2025 03:35:42 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro.org; s=google; t=1736163341; x=1736768141; 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=CxVlOv7MRs9GINJz5QT9QdIbM+9oFglvGClM+Y848Rs=; b=yReKwwIlTTKtclaM8Dxg9zx6LJLkPXbM8gtmgYZx3Yf41JwnRr8mI6AOTXnzFFhEOn +hBTFAmmRjbbWKcLKRGk7q30E7c0XQjKEH8J6B10dI6APZgjHIhUmVCSqZfPr9OWKPMk AQaW3r/Nmkzb2SmYG5jbFmX9KA+P9mzfHDQG1Ysc5/RfX+Mol4wd8NnY+DYboeu0HAO4 VooCyzlKak5seO2l0tPk1nGljqv5MG0TNFdPib+z1miNsBQsmKfEjRaOM4l+D5MPaJfi FZVbqQZOu+WxvJEmrCDzMBPQGFZ25I1+cmxnZgc4UBoC70H83u3c40KrWIhmQvXjS1Y8 QJag== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1736163341; x=1736768141; h=in-reply-to:content-disposition:mime-version:references:message-id :subject:cc:to:from:date:x-gm-message-state:from:to:cc:subject:date :message-id:reply-to; bh=CxVlOv7MRs9GINJz5QT9QdIbM+9oFglvGClM+Y848Rs=; b=dKq+yanZkRK5LwaB2Eghh8oSALoFzJNHStsL6KUDsOD/mVhE7ttwXCFZEvv3KYYaIk 3FdRkr2Y6XK1R7+1JzvfPYLG/N4czU7ZuyX5VqmmkpbBhOrbgvUbkR766Znz/+UhhD6l HMwX2+3AlJgEWcUeH8JldlYORAvzEwjy9Wp3fxBEtMQxo1pgAfPkorch/iI4qeDYpCbr PRaKZgxbOJAgYQKeEf04+5vUc7n9eD6Qgmn7PaxaNNsZvrQIUNr7PuToU1uyclUu5s3J nFFoUMKUGONWuqvGtVYcmUSm0KW/0R53LP9oTZfUoS8LfmVs1fz7cIa1IpRwxFci4Wcc 1RWg== X-Forwarded-Encrypted: i=1; AJvYcCWVE2uyI6MSS7FNejfxTdPyTaDELWe0RSa8lf5IDBHZAZLbDiXFiGrHz3CvN4TqKvFaYumZciW7gjKrhCE=@vger.kernel.org X-Gm-Message-State: AOJu0YzMoMD5Ur4NcCEQLf9QAtzE57MYIllcJ2XD79BSNlIaBkPPKGRW d1k0V/zp0zAo8/MXyeT2uiGvUHU6jf3xuITSk2Z0foe6SOLgVVlbY+La/ohv4Ts= X-Gm-Gg: ASbGncubGGoT7B5oQDjc6W2p53yHP4TJgzqh02JDnErqlHQiAe97Go3eT/OtN2Ov4e4 xjftufz2hDo2MgWYpytEqqJtPYGIBNk/O9C0wkmTm6sVvAxoXSx6rYBTu/ONipSIDbKUXrzqfKE /dnFnyX4tsPT2fWjy0RKux6L+7iEwHgdkRNnU1HP9zmUIk5hqmr+TJBakKjsCNS8n75qYLKlS7N CDFG3Qjak1OXkoAhR4DLxSciTIjBqiDYGM7fp9A+uVonYdF65eWtVwPMgg0nA== X-Google-Smtp-Source: AGHT+IE3U13MuikEBJhPkofq9DztLszvAkOREJWx5lHij9sjXdSLNKb8luysFS8kXjLfs0Q3ROwVhQ== X-Received: by 2002:a5d:5f56:0:b0:386:1cd3:8a00 with SMTP id ffacd0b85a97d-38a223f5b41mr56111928f8f.40.1736163341471; Mon, 06 Jan 2025 03:35:41 -0800 (PST) Received: from localhost ([196.207.164.177]) by smtp.gmail.com with ESMTPSA id ffacd0b85a97d-38a1c8474a9sm46997867f8f.52.2025.01.06.03.35.40 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Mon, 06 Jan 2025 03:35:40 -0800 (PST) Date: Mon, 6 Jan 2025 14:35:37 +0300 From: Dan Carpenter To: Christophe JAILLET Cc: Theodore Ts'o , Andreas Dilger , Alex Tomas , Eric Sandeen , "Aneesh Kumar K.V" , linux-kernel@vger.kernel.org, kernel-janitors@vger.kernel.org, Andreas Dilger , linux-ext4@vger.kernel.org Subject: Re: [PATCH 1/3] ext4: Fix an error handling path in ext4_mb_init_cache() Message-ID: <9383bdd6-ac04-4a14-aec1-bb65b67ace75@stanley.mountain> References: <3921e725586edaca611fd3de388f917e959dc85d.1735912719.git.christophe.jaillet@wanadoo.fr> 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: <3921e725586edaca611fd3de388f917e959dc85d.1735912719.git.christophe.jaillet@wanadoo.fr> On Fri, Jan 03, 2025 at 02:59:16PM +0100, Christophe JAILLET wrote: > 'bhs' is an un-initialized pointer. > If 'groups_per_page' == 1, 'bh' is assigned its address. > > Then, in the for loop below, if we early exit, either because > "group >= ngroups" or if ext4_get_group_info() fails, then it is still left > un-initialized. > > It can then be used. > NULL tests could fail and lead to unexpected behavior. Also, should the > error handling path be called, brelse() would be passed a potentially > invalid value. > > Better safe than sorry, just make sure it is correctly initialized to NULL. > > Fixes: c9de560ded61 ("ext4: Add multi block allocator for ext4") > Signed-off-by: Christophe JAILLET > --- > Compile tested only. > > The scenario looks possible, but I don't know if it can really happen... A pointer to the stack can't ever equal the address of the heap so this can't happen and it should not have a Fixes tag. Setting the pointer to NULL probably silences a static checker warning and these days everyone automatically zeroes stack data so it doesn't affect the compiled code. However generally we generally say that we should fix the checker instead. I've thought about this in Smatch for a while, and I think what I would do is say that kmalloc() returns memory that is unique. Smatch tracks if variables are equal to each other and unique variables wouldn't be equal to anything that came earlier. But I haven't actually tried to implement this. regards, dan carpenter