[LTP] [PATCH] lib: tst_detach_device_by_fd() set dev_fd to -1
Li Wang
liwang@redhat.com
Tue Oct 14 10:57:16 CEST 2025
On Tue, Oct 7, 2025 at 7:26 PM Cyril Hrubis <chrubis@suse.cz> wrote:
> Apparently I tend to forget that tst_detach_device_by_fd() closes the
> file descriptor. This change makes it more obvious by chaning the fd to
> a pointer and also sets the dev_fd to -1, which matches the SAFE_CLOSE()
> behavior.
>
It took me a while to think why we need to do this since we already had:
https://github.com/linux-test-project/ltp/commit/c02d8ddc5f9d312ca5c384317141e213412a5c42
However, this patch makes sense because:
After the tst_detach_device_by_fd() call, dev_fd is silently closed,
but the variable in the caller still retains its old value. If the caller
attempts to use it again, getting use-after-close may cause problems.
Therefore, reset it to -1 to prevent accidental use of a closed fd.
So maybe we could explicitly add the above explanation in the description
when merging?
Anyway:
Reviewed-by: Li Wang <liwang@redhat.com>
>
> Signed-off-by: Cyril Hrubis <chrubis@suse.cz>
> ---
> include/tst_device.h | 4 ++--
> lib/tst_device.c | 21 ++++++++++---------
> testcases/kernel/syscalls/ioctl/ioctl09.c | 2 +-
> .../kernel/syscalls/ioctl/ioctl_loop01.c | 6 +++---
> .../kernel/syscalls/ioctl/ioctl_loop02.c | 2 +-
> .../kernel/syscalls/ioctl/ioctl_loop04.c | 2 +-
> .../kernel/syscalls/ioctl/ioctl_loop06.c | 4 ++--
> .../kernel/syscalls/ioctl/ioctl_loop07.c | 4 ++--
> 8 files changed, 23 insertions(+), 22 deletions(-)
>
> diff --git a/include/tst_device.h b/include/tst_device.h
> index 898335b16..252942f8b 100644
> --- a/include/tst_device.h
> +++ b/include/tst_device.h
> @@ -77,10 +77,10 @@ uint64_t tst_get_device_size(const char *dev_path);
> * it is up to caller to open it again for further usage.
> *
> * @dev_path Path to the loop device e.g. /dev/loop0
> - * @dev_fd a open fd for the loop device
> + * @dev_fd An open fd for the loop device, set to -1 after the completion.
> * @return Zero on succes, non-zero otherwise.
> */
> -int tst_detach_device_by_fd(const char *dev_path, int dev_fd);
> +int tst_detach_device_by_fd(const char *dev_path, int *dev_fd);
>
> /*
> * Detaches a file from a loop device.
> diff --git a/lib/tst_device.c b/lib/tst_device.c
> index 2364df052..6ae914720 100644
> --- a/lib/tst_device.c
> +++ b/lib/tst_device.c
> @@ -239,9 +239,9 @@ uint64_t tst_get_device_size(const char *dev_path)
> return size/1024/1024;
> }
>
> -int tst_detach_device_by_fd(const char *dev, int dev_fd)
> +int tst_detach_device_by_fd(const char *dev, int *dev_fd)
> {
> - int ret, i;
> + int ret, i, retval = 1;
>
> /* keep trying to clear LOOPDEV until we get ENXIO, a quick
> succession
> * of attach/detach might not give udev enough time to complete
> @@ -250,19 +250,18 @@ int tst_detach_device_by_fd(const char *dev, int
> dev_fd)
> * device is detached only after last close.
> */
> for (i = 0; i < 40; i++) {
> - ret = ioctl(dev_fd, LOOP_CLR_FD, 0);
> + ret = ioctl(*dev_fd, LOOP_CLR_FD, 0);
>
> if (ret && (errno == ENXIO)) {
> - SAFE_CLOSE(NULL, dev_fd);
> - return 0;
> + retval = 0;
> + goto exit;
> }
>
> if (ret && (errno != EBUSY)) {
> tst_resm(TWARN,
> "ioctl(%s, LOOP_CLR_FD, 0) unexpectedly
> failed with: %s",
> dev, tst_strerrno(errno));
> - SAFE_CLOSE(NULL, dev_fd);
> - return 1;
> + goto exit;
> }
>
> usleep(50000);
> @@ -270,8 +269,10 @@ int tst_detach_device_by_fd(const char *dev, int
> dev_fd)
>
> tst_resm(TWARN,
> "ioctl(%s, LOOP_CLR_FD, 0) no ENXIO for too long", dev);
> - SAFE_CLOSE(NULL, dev_fd);
> - return 1;
> +exit:
> + SAFE_CLOSE(NULL, *dev_fd);
> + *dev_fd = -1;
> + return retval;
> }
>
> int tst_detach_device(const char *dev)
> @@ -284,7 +285,7 @@ int tst_detach_device(const char *dev)
> return 1;
> }
>
> - ret = tst_detach_device_by_fd(dev, dev_fd);
> + ret = tst_detach_device_by_fd(dev, &dev_fd);
> return ret;
> }
>
> diff --git a/testcases/kernel/syscalls/ioctl/ioctl09.c
> b/testcases/kernel/syscalls/ioctl/ioctl09.c
> index f86355f2c..262d6fcab 100644
> --- a/testcases/kernel/syscalls/ioctl/ioctl09.c
> +++ b/testcases/kernel/syscalls/ioctl/ioctl09.c
> @@ -76,7 +76,7 @@ static void verify_ioctl(void)
> check_partition(1, true);
> check_partition(2, true);
>
> - tst_detach_device_by_fd(dev_path, dev_fd);
> + tst_detach_device_by_fd(dev_path, &dev_fd);
> dev_fd = SAFE_OPEN(dev_path, O_RDWR);
> attach_flag = 0;
> }
> diff --git a/testcases/kernel/syscalls/ioctl/ioctl_loop01.c
> b/testcases/kernel/syscalls/ioctl/ioctl_loop01.c
> index cb1b506d2..4ecc93b1e 100644
> --- a/testcases/kernel/syscalls/ioctl/ioctl_loop01.c
> +++ b/testcases/kernel/syscalls/ioctl/ioctl_loop01.c
> @@ -99,13 +99,14 @@ static void verify_ioctl_loop(void)
> TST_ASSERT_INT(autoclear_path, 0);
> TST_ASSERT_STR(backing_path, backing_file_path);
>
> + dev_fd = SAFE_OPEN(dev_path, O_RDWR);
> +
> check_loop_value(SET_FLAGS, GET_FLAGS, 1);
>
> tst_res(TINFO, "Test flag can be clear");
> check_loop_value(0, LO_FLAGS_PARTSCAN, 0);
>
> - tst_detach_device_by_fd(dev_path, dev_fd);
> - dev_fd = SAFE_OPEN(dev_path, O_RDWR);
> + tst_detach_device_by_fd(dev_path, &dev_fd);
>
> attach_flag = 0;
> }
> @@ -122,7 +123,6 @@ static void setup(void)
> sprintf(sys_loop_partpath, "/sys/block/loop%d/loop%dp1", dev_num,
> dev_num);
> backing_file_path = tst_tmpdir_genpath("test.img");
> sprintf(loop_partpath, "%sp1", dev_path);
> - dev_fd = SAFE_OPEN(dev_path, O_RDWR);
> }
>
> static void cleanup(void)
> diff --git a/testcases/kernel/syscalls/ioctl/ioctl_loop02.c
> b/testcases/kernel/syscalls/ioctl/ioctl_loop02.c
> index 6026af1e2..10776d0b6 100644
> --- a/testcases/kernel/syscalls/ioctl/ioctl_loop02.c
> +++ b/testcases/kernel/syscalls/ioctl/ioctl_loop02.c
> @@ -100,7 +100,7 @@ static void verify_ioctl_loop(unsigned int n)
> }
>
> SAFE_CLOSE(file_fd);
> - tst_detach_device_by_fd(dev_path, dev_fd);
> + tst_detach_device_by_fd(dev_path, &dev_fd);
> dev_fd = SAFE_OPEN(dev_path, O_RDWR);
> attach_flag = 0;
> }
> diff --git a/testcases/kernel/syscalls/ioctl/ioctl_loop04.c
> b/testcases/kernel/syscalls/ioctl/ioctl_loop04.c
> index 839648f26..59f9de643 100644
> --- a/testcases/kernel/syscalls/ioctl/ioctl_loop04.c
> +++ b/testcases/kernel/syscalls/ioctl/ioctl_loop04.c
> @@ -60,7 +60,7 @@ static void verify_ioctl_loop(void)
> TST_ASSERT_INT(sys_loop_sizepath, NEW_SIZE/512);
>
> SAFE_CLOSE(file_fd);
> - tst_detach_device_by_fd(dev_path, dev_fd);
> + tst_detach_device_by_fd(dev_path, &dev_fd);
> dev_fd = SAFE_OPEN(dev_path, O_RDWR);
> unlink("test.img");
> attach_flag = 0;
> diff --git a/testcases/kernel/syscalls/ioctl/ioctl_loop06.c
> b/testcases/kernel/syscalls/ioctl/ioctl_loop06.c
> index 35e9e79e9..0a9618d00 100644
> --- a/testcases/kernel/syscalls/ioctl/ioctl_loop06.c
> +++ b/testcases/kernel/syscalls/ioctl/ioctl_loop06.c
> @@ -58,7 +58,7 @@ static void verify_ioctl_loop(unsigned int n)
> if (TST_RET == 0) {
> tst_res(TFAIL, "Set block size succeed unexpectedly");
> if (tcases[n].ioctl_flag == LOOP_CONFIGURE) {
> - tst_detach_device_by_fd(dev_path, dev_fd);
> + tst_detach_device_by_fd(dev_path, &dev_fd);
> dev_fd = SAFE_OPEN(dev_path, O_RDWR);
> }
> return;
> @@ -88,7 +88,7 @@ static void run(unsigned int n)
> return;
> }
> if (attach_flag) {
> - tst_detach_device_by_fd(dev_path, dev_fd);
> + tst_detach_device_by_fd(dev_path, &dev_fd);
> dev_fd = SAFE_OPEN(dev_path, O_RDWR);
> attach_flag = 0;
> }
> diff --git a/testcases/kernel/syscalls/ioctl/ioctl_loop07.c
> b/testcases/kernel/syscalls/ioctl/ioctl_loop07.c
> index be136fe0d..0e05b2021 100644
> --- a/testcases/kernel/syscalls/ioctl/ioctl_loop07.c
> +++ b/testcases/kernel/syscalls/ioctl/ioctl_loop07.c
> @@ -70,7 +70,7 @@ static void verify_ioctl_loop(unsigned int n)
> loopinfoget.lo_sizelimit,
> tc->set_sizelimit);
> /*Reset*/
> if (tc->ioctl_flag == LOOP_CONFIGURE) {
> - tst_detach_device_by_fd(dev_path, dev_fd);
> + tst_detach_device_by_fd(dev_path, &dev_fd);
> dev_fd = SAFE_OPEN(dev_path, O_RDWR);
> } else {
> loopinfo.lo_sizelimit = 0;
> @@ -99,7 +99,7 @@ static void run(unsigned int n)
> return;
> }
> if (attach_flag) {
> - tst_detach_device_by_fd(dev_path, dev_fd);
> + tst_detach_device_by_fd(dev_path, &dev_fd);
> dev_fd = SAFE_OPEN(dev_path, O_RDWR);
> attach_flag = 0;
> }
> --
> 2.49.1
>
>
> --
> Mailing list info: https://lists.linux.it/listinfo/ltp
>
>
--
Regards,
Li Wang
More information about the ltp
mailing list