Bug 213041 - opening a file with O_DIRECT on a file system that does not support it will leave an empty file
Summary: opening a file with O_DIRECT on a file system that does not support it will l...
Status: NEW
Alias: None
Product: File System
Classification: Unclassified
Component: VFS (show other bugs)
Hardware: All Linux
: P1 high
Assignee: fs_vfs
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2021-05-12 13:42 UTC by Colin Ian King
Modified: 2021-05-12 13:42 UTC (History)
0 users

See Also:
Kernel Version: 5.13 .. 3.13 (and before)
Subsystem:
Regression: No
Bisected commit-id:


Attachments

Description Colin Ian King 2021-05-12 13:42:59 UTC
files created with O_DIRECT on file systems that don't support this leave empty file behind after an open() -EINVAL failure.

Testcase:

1. create a minix file system and mount it
2. open a file on the file system with O_RDWR | O_CREAT | O_TRUNC | O_DIRECT
3. open fails with -EINVAL but leaves an empty file behind.  All other open() failures don't leave the failed open files behind.

I've tested this from 3.13 to current 5.13-rc1 and the issue is in all the kernels.

Reproducer:

#define _GNU_SOURCE

#include <sys/types.h>
#include <sys/stat.h>
#include <fcntl.h>
#include <stdio.h>
#include <unistd.h>
#include <string.h>
#include <errno.h>

int main(void)
{
        int fd;
        struct stat statbuf;
        const char filename[] = "/mnt/test";

        fd = open(filename, O_RDWR | O_CREAT | O_TRUNC | O_DIRECT, 0600);
        if (fd < 0)
                printf("open direct failed, errno=%d (%s)\n", errno, strerror(errno));

        if (stat(filename, &statbuf) == 0)
                printf("stat worked, eh? file open failure should not leave empty file\n");

        return 0;
}


In 5.13-rc1 the issue is clear, in fs/open.c, do_dentry_open() we have:

        f->f_mode |= FMODE_LSEEK | FMODE_PREAD | FMODE_PWRITE;
        if (!open)
                open = f->f_op->open;
        if (open) {
                error = open(inode, f);
                if (error)
                        goto cleanup_all;
        }
        f->f_mode |= FMODE_OPENED;
        if ((f->f_mode & (FMODE_READ | FMODE_WRITE)) == FMODE_READ)
                i_readcount_inc(inode);
        if ((f->f_mode & FMODE_READ) &&
             likely(f->f_op->read || f->f_op->read_iter))
                f->f_mode |= FMODE_CAN_READ;
        if ((f->f_mode & FMODE_WRITE) &&
             likely(f->f_op->write || f->f_op->write_iter))
                f->f_mode |= FMODE_CAN_WRITE;

        f->f_write_hint = WRITE_LIFE_NOT_SET;
        f->f_flags &= ~(O_CREAT | O_EXCL | O_NOCTTY | O_TRUNC);

        file_ra_state_init(&f->f_ra, f->f_mapping->host->i_mapping);

        /* NB: we're sure to have correct a_ops only after f_op->open */
        if (f->f_flags & O_DIRECT) {
                if (!f->f_mapping->a_ops || !f->f_mapping->a_ops->direct_IO)
                        return -EINVAL;
        }

..so the open with O_DIRECT performs the open, then the O_DIRECT direct_IO ops are checked and the function bails out with -EINVAL without the file cleanup.

Issue was detected with stress-ng --hdd 1 --hdd-opts direct --temp-path /tmp/mnt on minix file system.

Note You need to log in before you can comment on or make changes to this bug.