On 10/16/18 4:43 AM, Carlos Maiolino wrote:
> Recently we had a bug report of xfs_io frozen on a file, which ended up
> being a pipe, and xfs_io was waiting for data on the other side of the
> pipe.
>
> Although xfs_io was not stuck due a bug itself, we can do better and
> check the file type before opening the file. xfs_io has very limited
> usage on pipes, so, just check and deny opening of FIFO files.
This seems a little too heavy-handed. Operating on pipes is probably
rare, but xfs_io is supposed to be a generic I/O tool.
Today I can do for example:
# mkfifo pipe
# xfs_io -c stat pipe
fd.path = "pipe"
fd.flags = non-sync,non-direct,read-write
stat.ino = 102077957
stat.type = fifo
stat.size = 0
stat.blocks = 0
What xfs_io command was stuck in particular? These all seem to be handled uh, at least without a hang, if not correctly:
# xfs_io -c "pread 0 4096" pipe
pread: Illegal seek
# xfs_io -c "pwrite 0 4096" pipe
pwrite: Illegal seek
# xfs_io -c "bmap" pipe
foreign file active, bmap command is for XFS filesystems only
(?!)
Oh, this one is interesting ;)
# xfs_io -c "fiemap" pipe
pipe:
xfs_io: ioctl(FS_IOC_FIEMAP) ["pipe"]: Structure needs cleaning
...
[149881.306316] XFS (dm-0): Internal error xfs_bmapi_read at line 3817 of file fs/xfs/libxfs/xfs_bmap.c. Caller xfs_file_iomap_begin+0x16c/0x9f0 [xfs]
(eek?)
Anyway, I wonder if we can be more targeted in denying pipes if necessary, maybe CMD_PIPE_OK or CMD_NO_PIPE if that makes sense?
-Eric
> Signed-off-by: Carlos Maiolino <cmaiolino@xxxxxxxxxx>
> ---
>
> I belive having a generic helper to check the file type may have other uses too,
> so I opted to make it a generic helper.
>
> include/libfrog.h | 2 ++
> io/open.c | 6 ++++++
> libfrog/util.c | 12 ++++++++++++
> 3 files changed, 20 insertions(+)
>
> diff --git a/include/libfrog.h b/include/libfrog.h
> index d33f0146..693d026d 100644
> --- a/include/libfrog.h
> +++ b/include/libfrog.h
> @@ -5,7 +5,9 @@
> */
> #ifndef __LIBFROG_UTIL_H_
> #define __LIBFROG_UTIL_H_
> +#include <sys/types.h>
>
> unsigned int log2_roundup(unsigned int i);
> +unsigned int check_file_type(char *name, mode_t mode);
>
> #endif /* __LIBFROG_UTIL_H_ */
> diff --git a/io/open.c b/io/open.c
> index 6ea3e9a2..25f44b64 100644
> --- a/io/open.c
> +++ b/io/open.c
> @@ -9,6 +9,7 @@
> #include "init.h"
> #include "io.h"
> #include "libxfs.h"
> +#include "libfrog.h"
>
> #ifndef __O_TMPFILE
> #if defined __alpha__
> @@ -59,6 +60,11 @@ openfile(
> int fd;
> int oflags;
>
> + if (check_file_type(path, S_IFIFO)) {
> + fprintf(stderr, _("xfs_io does not work on FIFO files\n"));
> + return -1;
> + }
> +
> oflags = flags & IO_READONLY ? O_RDONLY : O_RDWR;
> if (flags & IO_APPEND)
> oflags |= O_APPEND;
> diff --git a/libfrog/util.c b/libfrog/util.c
> index ff935184..de0b8542 100644
> --- a/libfrog/util.c
> +++ b/libfrog/util.c
> @@ -5,6 +5,9 @@
> */
> #include "platform_defs.h"
> #include "libfrog.h"
> +#include <sys/types.h>
> +#include <sys/stat.h>
> +#include <unistd.h>
>
> /*
> * libfrog is a collection of miscellaneous userspace utilities.
> @@ -22,3 +25,12 @@ log2_roundup(unsigned int i)
> }
> return rval;
> }
> +
> +unsigned int
> +check_file_type(char *name, mode_t mode)
> +{
> + struct stat sb;
> +
> + lstat(name, &sb);
> + return (sb.st_mode & mode);
> +}
>
