From: Martin Wilck <mwilck@xxxxxxxx> The code in find_unused_loop_device() goes through circles to get an unused device, but it takes no care not to race with a different process opening the same loop device. Use the once-opened loop device for setup immediately instead of closing and re-opening it. While at it, simplify the code somewhat. Reviewed-by: Benjamin Marzinski <bmarzins@xxxxxxxxxx> Signed-off-by: Martin Wilck <mwilck@xxxxxxxx> --- kpartx/kpartx.c | 4 +-- kpartx/lopart.c | 83 ++++++++++++++++++++----------------------------- kpartx/lopart.h | 3 +- 3 files changed, 35 insertions(+), 55 deletions(-) diff --git a/kpartx/kpartx.c b/kpartx/kpartx.c index 7bc6454..3c49999 100644 --- a/kpartx/kpartx.c +++ b/kpartx/kpartx.c @@ -359,9 +359,7 @@ main(int argc, char **argv){ exit (0); if (!loopdev) { - loopdev = find_unused_loop_device(); - - if (set_loop(loopdev, rpath, 0, &ro)) { + if (set_loop(&loopdev, rpath, 0, &ro)) { fprintf(stderr, "can't set up loop\n"); exit (1); } diff --git a/kpartx/lopart.c b/kpartx/lopart.c index 2661940..7041ddf 100644 --- a/kpartx/lopart.c +++ b/kpartx/lopart.c @@ -39,24 +39,6 @@ #define LOOP_CTL_GET_FREE 0x4C82 #endif -static char * -xstrdup (const char *s) -{ - char *t; - - if (s == NULL) - return NULL; - - t = strdup (s); - - if (t == NULL) { - fprintf(stderr, "not enough memory"); - exit(1); - } - - return t; -} - #define SIZE(a) (sizeof(a)/sizeof(a[0])) char *find_loop_by_file(const char *filename) @@ -157,9 +139,9 @@ char *find_loop_by_file(const char *filename) return found; } -char *find_unused_loop_device(void) +static char *find_unused_loop_device(int mode, int *loop_fd) { - char dev[21], *next_loop_dev = NULL; + char dev[21]; int fd, next_loop = 0, somedev = 0, someloop = 0, loop_known = 0; int next_loop_fd; struct stat statbuf; @@ -168,45 +150,48 @@ char *find_unused_loop_device(void) next_loop_fd = open("/dev/loop-control", O_RDWR); if (next_loop_fd < 0) - return NULL; + goto no_loop_fd; - if (!(fstat(next_loop_fd, &statbuf) == 0 && S_ISCHR(statbuf.st_mode))) { - close(next_loop_fd); - return NULL; - } + if (!(fstat(next_loop_fd, &statbuf) == 0 && S_ISCHR(statbuf.st_mode))) + goto nothing_found; - while (next_loop_dev == NULL) { + for (;;) { next_loop = ioctl(next_loop_fd, LOOP_CTL_GET_FREE); - if (next_loop < 0) { - close(next_loop_fd); - return NULL; - } + if (next_loop < 0) + goto nothing_found; + sprintf(dev, "/dev/loop%d", next_loop); - fd = open (dev, O_RDONLY); + fd = open (dev, mode); if (fd >= 0) { if (fstat (fd, &statbuf) == 0 && S_ISBLK(statbuf.st_mode)) { somedev++; if(ioctl (fd, LOOP_GET_STATUS, &loopinfo) == 0) someloop++; /* in use */ - else if (errno == ENXIO) - next_loop_dev = xstrdup(dev); + else if (errno == ENXIO) { + char *name = strdup(dev); + + if (name == NULL) + close(fd); + else + *loop_fd = fd; + close(next_loop_fd); + return name; + } } close (fd); /* continue trying as long as devices exist */ - continue; - } - break; + } else + break; } +nothing_found: close(next_loop_fd); - if (next_loop_dev) - return next_loop_dev; - +no_loop_fd: /* Nothing found. Why not? */ if ((procdev = fopen(PROC_DEVICES, "r")) != NULL) { char line[100]; @@ -248,10 +233,10 @@ char *find_unused_loop_device(void) return NULL; } -int set_loop(const char *device, const char *file, int offset, int *loopro) +int set_loop(char **device, const char *file, int offset, int *loopro) { struct loop_info loopinfo; - int fd, ffd, mode; + int fd = -1, ret = 1, ffd, mode; mode = (*loopro ? O_RDONLY : O_RDWR); @@ -266,9 +251,9 @@ int set_loop(const char *device, const char *file, int offset, int *loopro) } } - if ((fd = open (device, mode)) < 0) { + *device = find_unused_loop_device(mode, &fd); + if (!*device) { close(ffd); - perror (device); return 1; } @@ -282,22 +267,20 @@ int set_loop(const char *device, const char *file, int offset, int *loopro) if (ioctl(fd, LOOP_SET_FD, (void*)(uintptr_t)(ffd)) < 0) { perror ("ioctl: LOOP_SET_FD"); - close (fd); - close (ffd); - return 1; + goto out; } if (ioctl (fd, LOOP_SET_STATUS, &loopinfo) < 0) { (void) ioctl (fd, LOOP_CLR_FD, 0); perror ("ioctl: LOOP_SET_STATUS"); - close (fd); - close (ffd); - return 1; + goto out; } + ret = 0; +out: close (fd); close (ffd); - return 0; + return ret; } int del_loop(const char *device) diff --git a/kpartx/lopart.h b/kpartx/lopart.h index d3bad10..c73ab23 100644 --- a/kpartx/lopart.h +++ b/kpartx/lopart.h @@ -1,5 +1,4 @@ extern int verbose; -extern int set_loop (const char *, const char *, int, int *); +extern int set_loop (char **, const char *, int, int *); extern int del_loop (const char *); -extern char * find_unused_loop_device (void); extern char * find_loop_by_file (const char *); -- 2.33.1 -- dm-devel mailing list dm-devel@xxxxxxxxxx https://listman.redhat.com/mailman/listinfo/dm-devel