From 67f1e027c27054e641584655020a417eaac9cb3a Mon Sep 17 00:00:00 2001
From: Lukas Prediger <lumip@lumip.de>
Date: Tue, 14 Sep 2021 00:09:42 +0100
Subject: [PATCH 1/3] drivers/cdrom: improved ioctl for media change detection
The current implementation of the CDROM_MEDIA_CHANGED ioctl relies on
global state, meaning that only one process can detect a disc change
while the ioctl call will return 0 for other calling processes afterwards
(see bug 213267).
This introduces a new cdrom ioctl, CDROM_TIMED_MEDIA_CHANGE, that
works by maintaining a timestamp of the last detected disc change instead
of a boolean flag: Processes calling this ioctl command can provide
a timestamp of the last disc change known to them and receive
an indication whether the disc was changed since then and the updated
timestamp.
I considered fixing the buggy behavior in the original
CDROM_MEDIA_CHANGED ioctl but that would require maintaining state
for each calling process in the kernel, which seems like a worse
solution than introducing this new ioctl.
Signed-off-by: Lukas Prediger <lumip@lumip.de>
Link: https://lore.kernel.org/all/20210912191207.74449-1-lumip@lumip.de
Signed-off-by: Phillip Potter <phil@philpotter.co.uk>
Link: https://lore.kernel.org/r/20210913230942.1188-1-phil@philpotter.co.uk
Signed-off-by: Jens Axboe <axboe@kernel.dk>
---
Documentation/cdrom/cdrom-standard.rst | 11 ++++
Documentation/userspace-api/ioctl/cdrom.rst | 3 ++
drivers/cdrom/cdrom.c | 59 +++++++++++++++++++--
include/linux/cdrom.h | 1 +
include/uapi/linux/cdrom.h | 19 +++++++
5 files changed, 89 insertions(+), 4 deletions(-)
diff --git a/Documentation/cdrom/cdrom-standard.rst b/Documentation/cdrom/cdrom-standard.rst
index 5845960ca382..52ea7b6b2fe8 100644
--- a/Documentation/cdrom/cdrom-standard.rst
+++ b/Documentation/cdrom/cdrom-standard.rst
@@ -907,6 +907,17 @@ commands can be identified by the underscores in their names.
specifies the slot for which the information is given. The special
value *CDSL_CURRENT* requests that information about the currently
selected slot be returned.
+`CDROM_TIMED_MEDIA_CHANGE`
+ Checks whether the disc has been changed since a user supplied time
+ and returns the time of the last disc change.
+
+ *arg* is a pointer to a *cdrom_timed_media_change_info* struct.
+ *arg->last_media_change* may be set by calling code to signal
+ the timestamp of the last known media change (by the caller).
+ Upon successful return, this ioctl call will set
+ *arg->last_media_change* to the latest media change timestamp (in ms)
+ known by the kernel/driver and set *arg->has_changed* to 1 if
+ that timestamp is more recent than the timestamp set by the caller.
`CDROM_DRIVE_STATUS`
Returns the status of the drive by a call to
*drive_status()*. Return values are defined in cdrom_drive_status_.
diff --git a/Documentation/userspace-api/ioctl/cdrom.rst b/Documentation/userspace-api/ioctl/cdrom.rst
index 3b4c0506de46..bac5bbf93ca0 100644
--- a/Documentation/userspace-api/ioctl/cdrom.rst
+++ b/Documentation/userspace-api/ioctl/cdrom.rst
@@ -54,6 +54,9 @@ are as follows:
CDROM_SELECT_SPEED Set the CD-ROM speed
CDROM_SELECT_DISC Select disc (for juke-boxes)
CDROM_MEDIA_CHANGED Check is media changed
+ CDROM_TIMED_MEDIA_CHANGE Check if media changed
+ since given time
+ (struct cdrom_timed_media_change_info)
CDROM_DRIVE_STATUS Get tray position, etc.
CDROM_DISC_STATUS Get disc type, etc.
CDROM_CHANGER_NSLOTS Get number of slots
diff --git a/drivers/cdrom/cdrom.c b/drivers/cdrom/cdrom.c
index bd2e5b1560f5..89a68457820a 100644
--- a/drivers/cdrom/cdrom.c
+++ b/drivers/cdrom/cdrom.c
@@ -344,6 +344,12 @@ static void cdrom_sysctl_register(void);
static LIST_HEAD(cdrom_list);
+static void signal_media_change(struct cdrom_device_info *cdi)
+{
+ cdi->mc_flags = 0x3; /* set media changed bits, on both queues */
+ cdi->last_media_change_ms = ktime_to_ms(ktime_get());
+}
+
int cdrom_dummy_generic_packet(struct cdrom_device_info *cdi,
struct packet_command *cgc)
{
@@ -616,6 +622,7 @@ int register_cdrom(struct gendisk *disk, struct cdrom_device_info *cdi)
ENSURE(cdo, generic_packet, CDC_GENERIC_PACKET);
cdi->mc_flags = 0;
cdi->options = CDO_USE_FFLAGS;
+ cdi->last_media_change_ms = ktime_to_ms(ktime_get());
if (autoclose == 1 && CDROM_CAN(CDC_CLOSE_TRAY))
cdi->options |= (int) CDO_AUTO_CLOSE;
@@ -1421,8 +1428,7 @@ static int cdrom_select_disc(struct cdrom_device_info *cdi, int slot)
cdi->ops->check_events(cdi, 0, slot);
if (slot == CDSL_NONE) {
- /* set media changed bits, on both queues */
- cdi->mc_flags = 0x3;
+ signal_media_change(cdi);
return cdrom_load_unload(cdi, -1);
}
@@ -1455,7 +1461,7 @@ static int cdrom_select_disc(struct cdrom_device_info *cdi, int slot)
slot = curslot;
/* set media changed bits on both queues */
- cdi->mc_flags = 0x3;
+ signal_media_change(cdi);
if ((ret = cdrom_load_unload(cdi, slot)))
return ret;
@@ -1521,7 +1527,7 @@ int media_changed(struct cdrom_device_info *cdi, int queue)
cdi->ioctl_events = 0;
if (changed) {
- cdi->mc_flags = 0x3; /* set bit on both queues */
+ signal_media_change(cdi);
ret |= 1;
cdi->media_written = 0;
}
@@ -2336,6 +2342,49 @@ static int cdrom_ioctl_media_changed(struct cdrom_device_info *cdi,
return ret;
}
+/*
+ * Media change detection with timing information.
+ *
+ * arg is a pointer to a cdrom_timed_media_change_info struct.
+ * arg->last_media_change may be set by calling code to signal
+ * the timestamp (in ms) of the last known media change (by the caller).
+ * Upon successful return, ioctl call will set arg->last_media_change
+ * to the latest media change timestamp known by the kernel/driver
+ * and set arg->has_changed to 1 if that timestamp is more recent
+ * than the timestamp set by the caller.
+ */
+static int cdrom_ioctl_timed_media_change(struct cdrom_device_info *cdi,
+ unsigned long arg)
+{
+ int ret;
+ struct cdrom_timed_media_change_info __user *info;
+ struct cdrom_timed_media_change_info tmp_info;
+
+ if (!CDROM_CAN(CDC_MEDIA_CHANGED))
+ return -ENOSYS;
+
+ info = (struct cdrom_timed_media_change_info __user *)arg;
+ cd_dbg(CD_DO_IOCTL, "entering CDROM_TIMED_MEDIA_CHANGE\n");
+
+ ret = cdrom_ioctl_media_changed(cdi, CDSL_CURRENT);
+ if (ret < 0)
+ return ret;
+
+ if (copy_from_user(&tmp_info, info, sizeof(tmp_info)) != 0)
+ return -EFAULT;
+
+ tmp_info.media_flags = 0;
+ if (tmp_info.last_media_change - cdi->last_media_change_ms < 0)
+ tmp_info.media_flags |= MEDIA_CHANGED_FLAG;
+
+ tmp_info.last_media_change = cdi->last_media_change_ms;
+
+ if (copy_to_user(info, &tmp_info, sizeof(*info)) != 0)
+ return -EFAULT;
+
+ return 0;
+}
+
static int cdrom_ioctl_set_options(struct cdrom_device_info *cdi,
unsigned long arg)
{
@@ -3313,6 +3362,8 @@ int cdrom_ioctl(struct cdrom_device_info *cdi, struct block_device *bdev,
return cdrom_ioctl_eject_sw(cdi, arg);
case CDROM_MEDIA_CHANGED:
return cdrom_ioctl_media_changed(cdi, arg);
+ case CDROM_TIMED_MEDIA_CHANGE:
+ return cdrom_ioctl_timed_media_change(cdi, arg);
case CDROM_SET_OPTIONS:
return cdrom_ioctl_set_options(cdi, arg);
case CDROM_CLEAR_OPTIONS:
diff --git a/include/linux/cdrom.h b/include/linux/cdrom.h
index c4fef00abdf3..0a89f111e00e 100644
--- a/include/linux/cdrom.h
+++ b/include/linux/cdrom.h
@@ -64,6 +64,7 @@ struct cdrom_device_info {
int for_data;
int (*exit)(struct cdrom_device_info *);
int mrw_mode_page;
+ __s64 last_media_change_ms;
};
struct cdrom_device_ops {
diff --git a/include/uapi/linux/cdrom.h b/include/uapi/linux/cdrom.h
index 6c34f6e2f1f7..804ff8d98f71 100644
--- a/include/uapi/linux/cdrom.h
+++ b/include/uapi/linux/cdrom.h
@@ -147,6 +147,8 @@
#define CDROM_NEXT_WRITABLE 0x5394 /* get next writable block */
#define CDROM_LAST_WRITTEN 0x5395 /* get last block written on disc */
+#define CDROM_TIMED_MEDIA_CHANGE 0x5396 /* get the timestamp of the last media change */
+
/*******************************************************
* CDROM IOCTL structures
*******************************************************/
@@ -295,6 +297,23 @@ struct cdrom_generic_command
};
};
+/* This struct is used by CDROM_TIMED_MEDIA_CHANGE */
+struct cdrom_timed_media_change_info {
+ __s64 last_media_change; /* Timestamp of the last detected media
+ * change in ms. May be set by caller,
+ * updated upon successful return of
+ * ioctl.
+ */
+ __u64 media_flags; /* Flags returned by ioctl to indicate
+ * media status.
+ */
+};
+#define MEDIA_CHANGED_FLAG 0x1 /* Last detected media change was more
+ * recent than last_media_change set by
+ * caller.
+ */
+/* other bits of media_flags available for future use */
+
/*
* A CD-ROM physical sector size is 2048, 2052, 2056, 2324, 2332, 2336,
* 2340, or 2352 bytes long.
From 9c03fee7e3fa01ca86d9d6195994cd9e6d24264b Mon Sep 17 00:00:00 2001
From: Phillip Potter <phil@philpotter.co.uk>
Date: Fri, 8 Oct 2021 00:17:20 +0100
Subject: [PATCH 2/3] cdrom: docs: reformat table in
Documentation/userspace-api/ioctl/cdrom.rst
Add extra space to the first column of ioctl values table, and also realign
the text in the second column, to fix a documentation build warning
introduced by: commit 67f1e027c270 ("drivers/cdrom: improved ioctl for
media change detection")
Suggested-by: Randy Dunlap <rdunlap@infradead.org>
Fixes: 67f1e027c270 ("drivers/cdrom: improved ioctl for media change detection")
Signed-off-by: Phillip Potter <phil@philpotter.co.uk>
Reviewed-by: Randy Dunlap <rdunlap@infradead.org>
Link: https://lore.kernel.org/r/20211007231720.31997-1-phil@philpotter.co.uk
Signed-off-by: Jens Axboe <axboe@kernel.dk>
---
Documentation/userspace-api/ioctl/cdrom.rst | 116 ++++++++++----------
1 file changed, 58 insertions(+), 58 deletions(-)
diff --git a/Documentation/userspace-api/ioctl/cdrom.rst b/Documentation/userspace-api/ioctl/cdrom.rst
index bac5bbf93ca0..682948fc88a3 100644
--- a/Documentation/userspace-api/ioctl/cdrom.rst
+++ b/Documentation/userspace-api/ioctl/cdrom.rst
@@ -13,64 +13,64 @@ in drivers/cdrom/cdrom.c and drivers/block/scsi_ioctl.c
ioctl values are listed in <linux/cdrom.h>. As of this writing, they
are as follows:
- ====================== ===============================================
- CDROMPAUSE Pause Audio Operation
- CDROMRESUME Resume paused Audio Operation
- CDROMPLAYMSF Play Audio MSF (struct cdrom_msf)
- CDROMPLAYTRKIND Play Audio Track/index (struct cdrom_ti)
- CDROMREADTOCHDR Read TOC header (struct cdrom_tochdr)
- CDROMREADTOCENTRY Read TOC entry (struct cdrom_tocentry)
- CDROMSTOP Stop the cdrom drive
- CDROMSTART Start the cdrom drive
- CDROMEJECT Ejects the cdrom media
- CDROMVOLCTRL Control output volume (struct cdrom_volctrl)
- CDROMSUBCHNL Read subchannel data (struct cdrom_subchnl)
- CDROMREADMODE2 Read CDROM mode 2 data (2336 Bytes)
- (struct cdrom_read)
- CDROMREADMODE1 Read CDROM mode 1 data (2048 Bytes)
- (struct cdrom_read)
- CDROMREADAUDIO (struct cdrom_read_audio)
- CDROMEJECT_SW enable(1)/disable(0) auto-ejecting
- CDROMMULTISESSION Obtain the start-of-last-session
- address of multi session disks
- (struct cdrom_multisession)
- CDROM_GET_MCN Obtain the "Universal Product Code"
- if available (struct cdrom_mcn)
- CDROM_GET_UPC Deprecated, use CDROM_GET_MCN instead.
- CDROMRESET hard-reset the drive
- CDROMVOLREAD Get the drive's volume setting
- (struct cdrom_volctrl)
- CDROMREADRAW read data in raw mode (2352 Bytes)
- (struct cdrom_read)
- CDROMREADCOOKED read data in cooked mode
- CDROMSEEK seek msf address
- CDROMPLAYBLK scsi-cd only, (struct cdrom_blk)
- CDROMREADALL read all 2646 bytes
- CDROMGETSPINDOWN return 4-bit spindown value
- CDROMSETSPINDOWN set 4-bit spindown value
- CDROMCLOSETRAY pendant of CDROMEJECT
- CDROM_SET_OPTIONS Set behavior options
- CDROM_CLEAR_OPTIONS Clear behavior options
- CDROM_SELECT_SPEED Set the CD-ROM speed
- CDROM_SELECT_DISC Select disc (for juke-boxes)
- CDROM_MEDIA_CHANGED Check is media changed
- CDROM_TIMED_MEDIA_CHANGE Check if media changed
- since given time
- (struct cdrom_timed_media_change_info)
- CDROM_DRIVE_STATUS Get tray position, etc.
- CDROM_DISC_STATUS Get disc type, etc.
- CDROM_CHANGER_NSLOTS Get number of slots
- CDROM_LOCKDOOR lock or unlock door
- CDROM_DEBUG Turn debug messages on/off
- CDROM_GET_CAPABILITY get capabilities
- CDROMAUDIOBUFSIZ set the audio buffer size
- DVD_READ_STRUCT Read structure
- DVD_WRITE_STRUCT Write structure
- DVD_AUTH Authentication
- CDROM_SEND_PACKET send a packet to the drive
- CDROM_NEXT_WRITABLE get next writable block
- CDROM_LAST_WRITTEN get last block written on disc
- ====================== ===============================================
+ ======================== ===============================================
+ CDROMPAUSE Pause Audio Operation
+ CDROMRESUME Resume paused Audio Operation
+ CDROMPLAYMSF Play Audio MSF (struct cdrom_msf)
+ CDROMPLAYTRKIND Play Audio Track/index (struct cdrom_ti)
+ CDROMREADTOCHDR Read TOC header (struct cdrom_tochdr)
+ CDROMREADTOCENTRY Read TOC entry (struct cdrom_tocentry)
+ CDROMSTOP Stop the cdrom drive
+ CDROMSTART Start the cdrom drive
+ CDROMEJECT Ejects the cdrom media
+ CDROMVOLCTRL Control output volume (struct cdrom_volctrl)
+ CDROMSUBCHNL Read subchannel data (struct cdrom_subchnl)
+ CDROMREADMODE2 Read CDROM mode 2 data (2336 Bytes)
+ (struct cdrom_read)
+ CDROMREADMODE1 Read CDROM mode 1 data (2048 Bytes)
+ (struct cdrom_read)
+ CDROMREADAUDIO (struct cdrom_read_audio)
+ CDROMEJECT_SW enable(1)/disable(0) auto-ejecting
+ CDROMMULTISESSION Obtain the start-of-last-session
+ address of multi session disks
+ (struct cdrom_multisession)
+ CDROM_GET_MCN Obtain the "Universal Product Code"
+ if available (struct cdrom_mcn)
+ CDROM_GET_UPC Deprecated, use CDROM_GET_MCN instead.
+ CDROMRESET hard-reset the drive
+ CDROMVOLREAD Get the drive's volume setting
+ (struct cdrom_volctrl)
+ CDROMREADRAW read data in raw mode (2352 Bytes)
+ (struct cdrom_read)
+ CDROMREADCOOKED read data in cooked mode
+ CDROMSEEK seek msf address
+ CDROMPLAYBLK scsi-cd only, (struct cdrom_blk)
+ CDROMREADALL read all 2646 bytes
+ CDROMGETSPINDOWN return 4-bit spindown value
+ CDROMSETSPINDOWN set 4-bit spindown value
+ CDROMCLOSETRAY pendant of CDROMEJECT
+ CDROM_SET_OPTIONS Set behavior options
+ CDROM_CLEAR_OPTIONS Clear behavior options
+ CDROM_SELECT_SPEED Set the CD-ROM speed
+ CDROM_SELECT_DISC Select disc (for juke-boxes)
+ CDROM_MEDIA_CHANGED Check is media changed
+ CDROM_TIMED_MEDIA_CHANGE Check if media changed
+ since given time
+ (struct cdrom_timed_media_change_info)
+ CDROM_DRIVE_STATUS Get tray position, etc.
+ CDROM_DISC_STATUS Get disc type, etc.
+ CDROM_CHANGER_NSLOTS Get number of slots
+ CDROM_LOCKDOOR lock or unlock door
+ CDROM_DEBUG Turn debug messages on/off
+ CDROM_GET_CAPABILITY get capabilities
+ CDROMAUDIOBUFSIZ set the audio buffer size
+ DVD_READ_STRUCT Read structure
+ DVD_WRITE_STRUCT Write structure
+ DVD_AUTH Authentication
+ CDROM_SEND_PACKET send a packet to the drive
+ CDROM_NEXT_WRITABLE get next writable block
+ CDROM_LAST_WRITTEN get last block written on disc
+ ======================== ===============================================
The information that follows was determined from reading kernel source
From bbc3925cf696422492ebdaba386e61450fa2294c Mon Sep 17 00:00:00 2001
From: luo penghao <luo.penghao@zte.com.cn>
Date: Thu, 21 Oct 2021 08:46:21 +0100
Subject: [PATCH 3/3] cdrom: Remove redundant variable and its assignment
Variable is not used in functions, and its assignment is redundant too.
So it should be deleted. Also the inner-most set of parentheses is no
longer needed.
The clang_analyzer complains as follows:
drivers/cdrom/cdrom.c:877: warning:
Although the value stored to 'ret' is used in the enclosing expression,
the value is never actually read from 'ret'.
Reported-by: Zeal Robot <zealci@zte.com.cn>
Signed-off-by: luo penghao <luo.penghao@zte.com.cn>
Link: https://lore.kernel.org/all/20211020024229.1036219-1-luo.penghao@zte.com.cn
Signed-off-by: Phillip Potter <phil@philpotter.co.uk>
Link: https://lore.kernel.org/r/20211021074621.901-1-phil@philpotter.co.uk
Signed-off-by: Jens Axboe <axboe@kernel.dk>
---
drivers/cdrom/cdrom.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/cdrom/cdrom.c b/drivers/cdrom/cdrom.c
index 89a68457820a..9877e413fce3 100644
--- a/drivers/cdrom/cdrom.c
+++ b/drivers/cdrom/cdrom.c
@@ -871,7 +871,7 @@ static void cdrom_mmc3_profile(struct cdrom_device_info *cdi)
{
struct packet_command cgc;
char buffer[32];
- int ret, mmc3_profile;
+ int mmc3_profile;
init_cdrom_command(&cgc, buffer, sizeof(buffer), CGC_DATA_READ);
@@ -881,7 +881,7 @@ static void cdrom_mmc3_profile(struct cdrom_device_info *cdi)
cgc.cmd[8] = sizeof(buffer); /* Allocation Length */
cgc.quiet = 1;
- if ((ret = cdi->ops->generic_packet(cdi, &cgc)))
+ if (cdi->ops->generic_packet(cdi, &cgc))
mmc3_profile = 0xffff;
else
mmc3_profile = (buffer[6] << 8) | buffer[7];