This patch just trades one undefined behaviour for another.

I'd open first and then do fstat.
Otherwise change your comment, because it isn't really enforcing anything
with a race condition between the stat and the open.

Sam

On Tue, Jan 5, 2016 at 11:50 AM, Ari Sundholm <[email protected]> wrote:

> We need to enforce that the opened file is a block device, as
> opening a file with the O_EXCL flag on and the O_CREATE flag off has
> undefined behavior unless the file is a block device.
>
> Signed-off-by: Ari Sundholm <[email protected]>
> ---
>  util-linux/blkdiscard.c | 14 +++++++++-----
>  1 file changed, 9 insertions(+), 5 deletions(-)
>
> diff --git a/util-linux/blkdiscard.c b/util-linux/blkdiscard.c
> index ace88a1..1b234fa 100644
> --- a/util-linux/blkdiscard.c
> +++ b/util-linux/blkdiscard.c
> @@ -38,7 +38,7 @@ int blkdiscard_main(int argc UNUSED_PARAM, char **argv)
>         uint64_t offset; /* Leaving these two variables out does not  */
>         uint64_t length; /* shrink code size and hampers readability. */
>         uint64_t range[2];
> -//     struct stat st;
> +       struct stat st;
>         int fd;
>
>         enum {
> @@ -51,11 +51,15 @@ int blkdiscard_main(int argc UNUSED_PARAM, char **argv)
>         opts = getopt32(argv, "o:l:s", &offset_str, &length_str);
>         argv += optind;
>
> +       /* We need to enforce that the opened file is a block device, as
> +        * opening a file with the O_EXCL flag on and the O_CREATE flag
> off has
> +        * undefined behavior unless the file is a block device.
> +        */
> +       xstat(argv[0], &st);
> +       if (!S_ISBLK(st.st_mode))
> +               bb_error_msg_and_die("%s: not a block device", argv[0]);
> +
>         fd = xopen(argv[0], O_RDWR|O_EXCL);
> -//Why bother, BLK[SEC]DISCARD will fail on non-blockdevs anyway?
> -//     xfstat(fd, &st);
> -//     if (!S_ISBLK(st.st_mode))
> -//             bb_error_msg_and_die("%s: not a block device", argv[0]);
>
>         offset = xatoull_sfx(offset_str, kMG_suffixes);
>
> --
> 1.9.1
>
>
>
> _______________________________________________
> busybox mailing list
> [email protected]
> http://lists.busybox.net/mailman/listinfo/busybox
>
_______________________________________________
busybox mailing list
[email protected]
http://lists.busybox.net/mailman/listinfo/busybox

Reply via email to