From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mail-wm1-f43.google.com (mail-wm1-f43.google.com [209.85.128.43]) (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 411F520F067 for ; Sun, 22 Feb 2026 19:11:04 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=209.85.128.43 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1771787466; cv=none; b=fnqs/bBdyA35g4cMNKxlmpmwJ15Qz4zqvGc7IPsoJHmiIZZRFDoXhQpIZ08CRb++SbvtE8BCojyI+jvjvT3OHLmFqiVbKVwj12vW3XMXUkHGqyxr7nyjm5SDz9FZyOlj40e0hDfUKP51ZwtlcX0tBBCeJ9v4Caiym0jSUWeiXME= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1771787466; c=relaxed/simple; bh=V1l1CX/zK9bIwPCgnDovsCEHX/sAXIvFxMCWvDiOrrI=; h=Date:From:To:Cc:Subject:Message-ID:References:MIME-Version: Content-Type:Content-Disposition:In-Reply-To; b=ePUy8mZTsqlf+uEIgMSCm0vodzbdoP4kvquKVeDwF4rZ4djwsi3z8XKsVG2r2qp4+5+603flEGqwKvfpWSMNyNx/ydoCS4CCiU5BczVlQMZtATE6z/7v+p6V/OYmW/SuPcBdwcRGNAtCIq6sQUqWKo0CN7iqp9Ly6PKVX+DauXc= 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.20230601.gappssmtp.com header.i=@philpotter-co-uk.20230601.gappssmtp.com header.b=K/RxR+/t; arc=none smtp.client-ip=209.85.128.43 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.20230601.gappssmtp.com header.i=@philpotter-co-uk.20230601.gappssmtp.com header.b="K/RxR+/t" Received: by mail-wm1-f43.google.com with SMTP id 5b1f17b1804b1-4838c15e3cbso30946785e9.3 for ; Sun, 22 Feb 2026 11:11:03 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=philpotter-co-uk.20230601.gappssmtp.com; s=20230601; t=1771787462; x=1772392262; 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=9M2IqeY0CH6mF/sOklYeD+yPA/um1ysL/LiXj1d82Vw=; b=K/RxR+/tRkaXWlwZy43ImjF6zrXrOZqw0/IjSPRqi+UchG0nKpnlJHzFxK6+wK3W+g Vl9bahyxuSZxrRv5st0YedVjPNOm7EK6WtIhwIWOMHuYsPQ1NKN6Aof3tMMI1Q+SOQSI 3G5/vYZhFjoxoMf4uBvt3+bnWweSI4GqagAklrwqc/KdW4sSLn8pC9MalC6ycaRB3a+W EmN3iuoE6BaN+fBt1458LXCdq+pahlXmnoZFqA6soTg9ifD79gPjpq402QcNlv55+2jX rxKpHnJxUjATaRcAqDKVDf8gQZKCQ59UL47qDRxj0znXPcP8ElT6Z1Wiff4Pgphg8BNo 4dDQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1771787462; x=1772392262; 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=9M2IqeY0CH6mF/sOklYeD+yPA/um1ysL/LiXj1d82Vw=; b=OfZqcLNoNohDWfPaFWimzy5PE3WoB+PAKnFroxuAsYCSGAAkC27Jii2nyjQ2wE597A Ko+KMwnShxMYsFnA1fS/hsqU4sTkpFZtNGF+c2wkppJZIYCJLHlRHMH2NkyqyrPd/drP edndJb95igBd5TgaoXdIbajepiEOJfMK3rJL8Mo3g1BjAIm9oLNszHiiSCUAXGDKD+B7 p25+mu5YlFzrjgeR6p1TzPsngzIZ72rtEDVJZrLrjHIhGywL4KBw+5HQonMecoNFppLZ KhWzAScFHrh1MmCpAMZMQpCenw27kjO9X5y0OHL4kIcbI9iuYvCXE0NKY4cn7yyulp2q 1uUA== X-Gm-Message-State: AOJu0Yy4i8AevOQp2miXJJoWFCA4UxEwuwL/+A4GZIpbO3ESDFQK6Bxk h5fQS+eTW5Kn504uu3NL6NnLGb3DhYORbsQQiaqUsTGpvcKf8LTaIRyxj1GBrLKhm/U= X-Gm-Gg: AZuq6aL511u5WTsrNiK4mnXLwGjX1t3wBrMCIHM95IoYGAfmUiNaDg7Z5iY4fqqi6I5 azt2FnPbsfx3mGWYjsY9ZcmOxrsudTyITL2RkSD5pZ2HLqv/Ej0UjueuA7ElGKqVVZOLJ3jfeJU 73WyJyLcLyYVtxvZdyjbJcHmhVam1XBnPCILCSmAlg7o6Zqpe8dj9XA5d1pQeecbcvFXhkornmZ C6csPBp+SGcmubaZkbPTuCI/ONdCvIiKLYrjGaN06gFhdX46/GkQ7dvHgCwOHRNZsXFqTsxP11P 6K6JrS8MulKja3/3YfPjCD+DKuzBNKgMXKSAJEe6R3q9OXIQxxTGwo0EwCz/aaJyGAJK7Q06QqE USKCG2zs65izi1KNpMWgU3P9ivD2K8CNOKDoK/nRBXCEWFI3znCrGzznvJtb04lIIObZxMWvCg8 JekHXbqsKVPX51ZdOV/ht/1+Go518IBHboAAmCJgD7yqw19l6ceSWufh2EzvHgApRdJc9i19/eT U9eTRFZQCo= X-Received: by 2002:a05:600c:6094:b0:47e:e8de:7420 with SMTP id 5b1f17b1804b1-483a962f0c5mr96087615e9.22.1771787462039; Sun, 22 Feb 2026 11:11:02 -0800 (PST) Received: from KernelVM (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 5b1f17b1804b1-483a8df83bcsm159051855e9.13.2026.02.22.11.11.01 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Sun, 22 Feb 2026 11:11:01 -0800 (PST) Date: Sun, 22 Feb 2026 19:11:00 +0000 From: Phillip Potter To: Felix Busch Cc: linux-kernel@vger.kernel.org, Phillip Potter Subject: Re: [PATCH] Added special upper bound check for the logical block address in mmc_ioctl_cdrom_read_data() Message-ID: References: <20260222-cdrom-additional-lba-check-v1-1-5f5e9f0c0fa4@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=us-ascii Content-Disposition: inline In-Reply-To: <20260222-cdrom-additional-lba-check-v1-1-5f5e9f0c0fa4@gmail.com> Hi Felix, Thank you for your patch firstly. First issue however is it doesn't conform to the required uniform-diff format such that it can be applied by tools such as 'git am'. The patch e-mail should use a subject of the form (for example): [PATCH] cdrom: short imperative description With a longer description such as the one you've supplied, imperatively, followed by your Signed-off-by tag. Please review this for additional guidance: https://docs.kernel.org/process/submitting-patches.html Onto the patch itself... On Sun, Feb 22, 2026 at 05:41:46PM +0100, Felix Busch wrote: > Signed-off-by: Felix Busch > --- > This patch contains an extra check on the comment > for an upper bound check for the logical block address in the > function mmc_ioctl_cdrom_read_data(). > > This web page: > http://www.o3one.org/hwdocs/cdrom_formats/scsi_programming.htm 'This webpage' is evidently a reference to the SCSI-2 spec, but this should be clearer in the description if it's being referenced. Its veracity should also be considered (although this information seems to appear verbatim on a number of sites). > > states that: > "Logical adressing of CD-ROM information may use any logical > block length. When the specified logical block length is an > exact divisor or integral multiple of the selected number > of bytes per CD-ROM sector, the device shall map (one to one) > the bytes transferred from CD-ROM sectors to the bytes of logical > blocks. For instance, if 2048 bytes are transferred from each > CD-ROM sector,.., and the logical block length is 512 bytes, then > each CD-ROM sector shall map to exactly four logical blocks." > > If the number of sectors on the CD drive is a multiple of the block > size, then, as I understand, it should be possible to perform a > simple check on the logical block address in this case. > > If the logical block address (lba value) is greater than > (logical_blocks-1) * blocksize, then it should not be possible > to read the next block, because if the lba value is greater > than this value, then it might try to read a full next block that > does not exist. > > Please let me know what you think. > Thank you very much. > --- > drivers/cdrom/cdrom.c | 11 +++++++++++ > 1 file changed, 11 insertions(+) > > diff --git a/drivers/cdrom/cdrom.c b/drivers/cdrom/cdrom.c > index 31ba1f8c1f78..0149813ef903 100644 > --- a/drivers/cdrom/cdrom.c > +++ b/drivers/cdrom/cdrom.c > @@ -2927,6 +2927,7 @@ static noinline int mmc_ioctl_cdrom_read_data(struct cdrom_device_info *cdi, > struct scsi_sense_hdr sshdr; > struct cdrom_msf msf; > int blocksize = 0, format = 0, lba; > + unsigned int cd_nr_sectors; This should be sector_t strictly speaking. > int ret; > > switch (cmd) { > @@ -2945,9 +2946,19 @@ static noinline int mmc_ioctl_cdrom_read_data(struct cdrom_device_info *cdi, > return -EFAULT; > lba = msf_to_lba(msf.cdmsf_min0, msf.cdmsf_sec0, msf.cdmsf_frame0); > /* FIXME: we need upper bound checking, too!! */ > + /* Lower bound check for logical block address. */ > if (lba < 0) > return -EINVAL; > > + cd_nr_sectors = cdi->disk->part0->bd_nr_sectors; There is a utility function for reading number of sectors. That (to me at least) seems preferable to accessing this field directly. > + /* A special case upper bound check. */ > + if (cd_nr_sectors % blocksize == 0) { > + unsigned int logical_blocks = cd_nr_sectors / blocksize; Again this should be sector_t. I'm not even sure this is correct though. We are taking the number of sectors and dividing it by the block size determined above based on read mode of RAW/1/2. Perhaps my eyes are just tired, but how does that give us the number of logical blocks? It seems this code confuses sector size with number of sectors unless I'm missing something. Has it been built/run/tested? > + > + if (lba > blocksize * (logical_blocks - 1)) > + return -EINVAL; > + } > + > cgc->buffer = kzalloc(blocksize, GFP_KERNEL); > if (cgc->buffer == NULL) > return -ENOMEM; > > --- > base-commit: 05f7e89ab9731565d8a62e3b5d1ec206485eeb0b > change-id: 20260222-cdrom-additional-lba-check-2c88d18599d0 > > Best regards, > -- > Felix Busch > Regards, Phil