From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mail-wm1-f41.google.com (mail-wm1-f41.google.com [209.85.128.41]) (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 A653B35AC1E for ; Fri, 5 Jun 2026 16:11:05 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=209.85.128.41 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1780675867; cv=none; b=Tpbinw0XAg6cmUAS9M91aYIEm7y6F757cvb1dfCn40qm7DZXhXr/PX6OCgVVlY/QwNlROx5Nk4UDZyR0kx86xi7SETqkgK8YsxS7odpUSx3TAjq3Y/R18aTPOJyO/7v/9CUBDvPVTSjN6qC4I1df2r0f2PhExhY0dEhj4vO3cjk= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1780675867; c=relaxed/simple; bh=uBujOcfv5xWvj+pYonY5gZc1x+0FPJU0wZNp6qLnbJU=; h=Date:From:To:Cc:Subject:Message-ID:References:MIME-Version: Content-Type:Content-Disposition:In-Reply-To; b=pvQiv5Ue0Yql8tRVqJ6C5Owsb6eCXuDZPs4u/4lXUJZ/GcIYZL8sPT8W3a5vqQDvibSeRa05qwZIZ8XDe+OmUXMKDlAztdiJwFxIexWUbsQy7tuQ+vl+64CHS/T42TiZmp8e/i+d7cKwRVcpyNoSypPz74yOLz+bdD8uD1iD1sM= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dmarc=none (p=none dis=none) header.from=philpotter.co.uk; spf=pass smtp.mailfrom=philpotter.co.uk; dkim=pass (2048-bit key) header.d=philpotter-co-uk.20251104.gappssmtp.com header.i=@philpotter-co-uk.20251104.gappssmtp.com header.b=oSSQCVL2; arc=none smtp.client-ip=209.85.128.41 Authentication-Results: smtp.subspace.kernel.org; dmarc=none (p=none dis=none) header.from=philpotter.co.uk Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=philpotter.co.uk Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=philpotter-co-uk.20251104.gappssmtp.com header.i=@philpotter-co-uk.20251104.gappssmtp.com header.b="oSSQCVL2" Received: by mail-wm1-f41.google.com with SMTP id 5b1f17b1804b1-4906869f0cbso24902305e9.1 for ; Fri, 05 Jun 2026 09:11:05 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=philpotter-co-uk.20251104.gappssmtp.com; s=20251104; t=1780675864; x=1781280664; darn=vger.kernel.org; h=in-reply-to:content-transfer-encoding:content-disposition :mime-version:references:message-id:subject:cc:to:from:date:from:to :cc:subject:date:message-id:reply-to; bh=DD2OtvhopM0ZNBJbb1UTE7FanzWiIQZFVIvMVfvCQn8=; b=oSSQCVL2A5Ufk/K4q3oYj6Lmg+SxkJpQKr2cpo6vhZKRSDJdqIp1mEqKo33YoSMSVF rCoS/+4UvM767cF8yr0FREEl4f5hXNUyAhowGqor92AIwt5ttI/haTDyGA8LcHksS7lY meVdhyCcgLHLtcL127Suj94ZWPlcUDl7tzNebM2KOKUM4lQDAtSK4Zit2BC21JQlqNO2 Knb/cNCl7kW07gtb/h/V7Syzo2XTzRAYSNq6Cq+aFuzEpldPuYcBj0mPoin4v5ICeen5 RHsclkWyXKM9/1O4isY1f5gc+rCUiOwz91jay6rWGaXc7epL4Ab4Pd4deYrA6kE2SKXL R9wA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20251104; t=1780675864; x=1781280664; h=in-reply-to:content-transfer-encoding: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=DD2OtvhopM0ZNBJbb1UTE7FanzWiIQZFVIvMVfvCQn8=; b=VqOfrLFGh5Yy1eXDR02Fyhke/4hHIESQS6Drlz/h0c2Ogss82ET7RmXym1qhKbH8EH onMV1ANKGG22DR9ugWSJOCBUBctsms7mWyrCNzEDGdE+QKR8OOmuGxxyMkdC7sv2IXFj rEsePBfHcsyfeD3SncPgBwLbrHIRaZxRKg9/L+RosTQaaaksDqUgn0Y3m7q68uxjarZY 1Zcg/Q7i26CkT83oYz5ErFdeq2CDl+dQTtlME+nsubA0swkZiG6rte0ciBlOHqbIU96K 4NIZr18qUe+JNLQXsf/E3oou9xkcCTJksEiiVqQ+sg2qBAY9jyhguTSuoid2Car9o0Yf i9xA== X-Gm-Message-State: AOJu0Yy7a5v5cXgtVEJ4JlQNj1g/q+xrQtbmpWXoSU/+cJrDOQCM1s4w xOFGdWOUi1XasilSs9FlnlkglBE8x4yuA2tsMGToFhrfrMGdFobwVUd6sTQuKaoLlkfpg8zF9Ew A5/fPbyQ= X-Gm-Gg: Acq92OFlXICJxD9Thc8clxSj2Ea+D5K/xrk07BQF40A5WfarmJXxGI+A5qCkXd2A+fe TR0GyIikpNtVN+3TkPfi+znqZzXVdxKHU4XeXuwG4mJ62rFuVU+ToJBafgvEIpw5LA3h21Q2asy Grdno0CfwCT614Z9rZgINu6lZnNuXVLypukCXWN4AffmDXKy/E0j/xX2Di8U8bRJowHwthD3+CL I0r1hqW4qwLbPdhXQ3kMCoL3jFWG05BX7AOBtRyqY9rT9sDM6SvdGdFIcRnmrpz/4HxU+jUb49O JMK1daErG8C3MpABKUYrtwPdJB2eM1kSY50cwaLYo4NcxFv0bJFXo+I3zQQ4QWwLfCe+A/6jh2c GumgCHAsgBfoGtl/I+rEEO5NBKZIQDYOLob5l9GOHKFJdoIT0YotL0ohcK9x2ha6riR+Fevcu3k 97PXQj5PMpyXzwiyh35jEH2WTGl0T5QTDQpYhooPrN1jWucLOguhRtWPeuxisetIgBkl5noXjXE 3z2IjsiT4ienklrSyza X-Received: by 2002:a05:600c:818c:b0:490:b0e1:2161 with SMTP id 5b1f17b1804b1-490c25b39fdmr80486575e9.2.1780675863895; Fri, 05 Jun 2026 09:11:03 -0700 (PDT) Received: from equinox (2.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.6.1.f.d.0.b.8.0.1.0.0.2.ip6.arpa. [2001:8b0:df16::2]) by smtp.gmail.com with ESMTPSA id ffacd0b85a97d-4601f2dcb13sm26422074f8f.2.2026.06.05.09.11.03 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Fri, 05 Jun 2026 09:11:03 -0700 (PDT) Date: Fri, 5 Jun 2026 17:11:01 +0100 From: Phillip Potter To: =?utf-8?Q?J=C4=99drzej?= Szoszorek Cc: linux-kernel@vger.kernel.org, phil@philpotter.co.uk Subject: Re: [PATCH v2] cdrom: use struct_size() in changer info allocation Message-ID: References: <20260604190828.128448-1-jedrzej.szoszo@gmail.com> 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=utf-8 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: <20260604190828.128448-1-jedrzej.szoszo@gmail.com> On Thu, Jun 04, 2026 at 09:08:28PM +0200, Jędrzej Szoszorek wrote: > Replace the obsolete `kmalloc_obj()` pattern with the > `kzalloc(struct_size(), ...)` idiom when allocating `struct cdrom_changer_info`. > > This change ensures inherent protection against integer overflow > vulnerabilities during the calculation of the allocation size, as > `struct_size()` safely computes the size of the structure combined with > its flexible array member. > > This addresses memory safety concerns without altering the driver's > logic or ABI, guaranteeing zero regressions for legacy user-space tools. > > Signed-off-by: Jędrzej Szoszorek > --- > - Dropped all cosmetic and formatting changes (churn) from v1. > - Dropped ioctl error code changes (-ENOSYS -> -ENOTTY) to prevent > any userspace ABI breakage. > - Dropped experimental per-device locking (mutex/spinlock) to avoid > TOCTOU races and hardware lockout risks. > - Kept only the critical memory safety fix (struct_size) > > drivers/cdrom/cdrom.c | 9 +++++---- > 1 file changed, 5 insertions(+), 4 deletions(-) > > diff --git a/drivers/cdrom/cdrom.c b/drivers/cdrom/cdrom.c > index 62934cf4b..31e662c8b 100644 > --- a/drivers/cdrom/cdrom.c > +++ b/drivers/cdrom/cdrom.c > @@ -276,6 +276,7 @@ > #include > #include > #include > +#include > > /* used to tell the module to turn on full debugging messages */ > static bool debug; > @@ -1341,7 +1342,7 @@ static int cdrom_slot_status(struct cdrom_device_info *cdi, int slot) > if (cdi->sanyo_slot) > return CDS_NO_INFO; > > - info = kmalloc_obj(*info); > + info = kzalloc(struct_size(info, slots, cdi->capacity), GFP_KERNEL); > if (!info) > return -ENOMEM; > > @@ -1370,7 +1371,7 @@ int cdrom_number_of_slots(struct cdrom_device_info *cdi) > /* cdrom_read_mech_status requires a valid value for capacity: */ > cdi->capacity = 0; > > - info = kmalloc_obj(*info); > + info = kzalloc(struct_size(info, slots, cdi->capacity), GFP_KERNEL); > if (!info) > return -ENOMEM; > > @@ -1429,7 +1430,7 @@ static int cdrom_select_disc(struct cdrom_device_info *cdi, int slot) > return cdrom_load_unload(cdi, -1); > } > > - info = kmalloc_obj(*info); > + info = kzalloc(struct_size(info, slots, cdi->capacity), GFP_KERNEL); > if (!info) > return -ENOMEM; > > @@ -2334,7 +2335,7 @@ static int cdrom_ioctl_media_changed(struct cdrom_device_info *cdi, > /* Prevent arg from speculatively bypassing the length check */ > arg = array_index_nospec(arg, cdi->capacity); > > - info = kmalloc_obj(*info); > + info = kzalloc(struct_size(info, slots, cdi->capacity), GFP_KERNEL); > if (!info) > return -ENOMEM; > > -- > 2.43.0 > Hi Jędrzej, Sorry, but this patch is just not correct. There is no flexible array member in 'struct cdrom_changer_info', which is what all four of these calls allocate. The slots array in that structure is sized from a constant as quoted below: > /* The SCSI spec says there could be 256 slots. */ > #define CDROM_MAX_SLOTS 256 The code thus incorrectly allocates this structure with the changes (which to me is just as relevant as whether that extra space is actually used or not). In my view, these calls are fine as they are and I see no need to replace them therefore, particularly for the reasons stated. There are no "memory safety concerns" as listed in your commit description. Regards, Phil