* [PATCH v4 3/3] fs: let filldir_t return bool instead of an error code
From: Jann Horn @ 2019-01-18 16:14 UTC (permalink / raw)
To: Richard Henderson, Ivan Kokshaysky, Matt Turner, Alexander Viro,
linux-fsdevel, Arnd Bergmann, jannh
Cc: Eric W. Biederman, Theodore Ts'o, Andreas Dilger, linux-alpha,
linux-kernel, Dave Chinner, Pavel Machek, linux-arch, linux-api
In-Reply-To: <20190118161440.220134-1-jannh@google.com>
As Al Viro pointed out, many filldir_t functions return error codes, but
all callers of filldir_t functions just check whether the return value is
non-zero (to determine whether to continue reading the directory); more
precise errors have to be signalled via struct dir_context.
Change all filldir_t functions to return bool instead of int.
Suggested-by: Al Viro <viro@zeniv.linux.org.uk>
Signed-off-by: Jann Horn <jannh@google.com>
---
arch/alpha/kernel/osf_sys.c | 12 +++----
fs/afs/dir.c | 30 +++++++++--------
fs/ecryptfs/file.c | 13 ++++----
fs/exportfs/expfs.c | 8 ++---
fs/fat/dir.c | 8 ++---
fs/gfs2/export.c | 6 ++--
fs/nfsd/nfs4recover.c | 8 ++---
fs/nfsd/vfs.c | 6 ++--
fs/ocfs2/dir.c | 10 +++---
fs/ocfs2/journal.c | 14 ++++----
fs/overlayfs/readdir.c | 24 +++++++-------
fs/readdir.c | 64 ++++++++++++++++++-------------------
fs/reiserfs/xattr.c | 20 ++++++------
fs/xfs/scrub/dir.c | 8 ++---
fs/xfs/scrub/parent.c | 4 +--
include/linux/fs.h | 10 +++---
16 files changed, 125 insertions(+), 120 deletions(-)
diff --git a/arch/alpha/kernel/osf_sys.c b/arch/alpha/kernel/osf_sys.c
index db1c2144d477..14e5ae0dac50 100644
--- a/arch/alpha/kernel/osf_sys.c
+++ b/arch/alpha/kernel/osf_sys.c
@@ -108,7 +108,7 @@ struct osf_dirent_callback {
int error;
};
-static int
+static bool
osf_filldir(struct dir_context *ctx, const char *name, int namlen,
loff_t offset, u64 ino, unsigned int d_type)
{
@@ -120,14 +120,14 @@ osf_filldir(struct dir_context *ctx, const char *name, int namlen,
buf->error = check_dirent_name(name, namlen);
if (unlikely(buf->error))
- return -EFSCORRUPTED;
+ return false;
buf->error = -EINVAL; /* only used if we fail */
if (reclen > buf->count)
- return -EINVAL;
+ return false;
d_ino = ino;
if (sizeof(d_ino) < sizeof(ino) && d_ino != ino) {
buf->error = -EOVERFLOW;
- return -EOVERFLOW;
+ return false;
}
if (buf->basep) {
if (put_user(offset, buf->basep))
@@ -144,10 +144,10 @@ osf_filldir(struct dir_context *ctx, const char *name, int namlen,
dirent = (void __user *)dirent + reclen;
buf->dirent = dirent;
buf->count -= reclen;
- return 0;
+ return true;
Efault:
buf->error = -EFAULT;
- return -EFAULT;
+ return false;
}
SYSCALL_DEFINE4(osf_getdirentries, unsigned int, fd,
diff --git a/fs/afs/dir.c b/fs/afs/dir.c
index 8a2562e3a316..84d74cc25127 100644
--- a/fs/afs/dir.c
+++ b/fs/afs/dir.c
@@ -26,10 +26,12 @@ static int afs_dir_open(struct inode *inode, struct file *file);
static int afs_readdir(struct file *file, struct dir_context *ctx);
static int afs_d_revalidate(struct dentry *dentry, unsigned int flags);
static int afs_d_delete(const struct dentry *dentry);
-static int afs_lookup_one_filldir(struct dir_context *ctx, const char *name, int nlen,
- loff_t fpos, u64 ino, unsigned dtype);
-static int afs_lookup_filldir(struct dir_context *ctx, const char *name, int nlen,
- loff_t fpos, u64 ino, unsigned dtype);
+static bool afs_lookup_one_filldir(struct dir_context *ctx, const char *name,
+ int nlen, loff_t fpos, u64 ino,
+ unsigned int dtype);
+static bool afs_lookup_filldir(struct dir_context *ctx, const char *name,
+ int nlen, loff_t fpos, u64 ino,
+ unsigned int dtype);
static int afs_create(struct inode *dir, struct dentry *dentry, umode_t mode,
bool excl);
static int afs_mkdir(struct inode *dir, struct dentry *dentry, umode_t mode);
@@ -493,7 +495,7 @@ static int afs_readdir(struct file *file, struct dir_context *ctx)
* - if afs_dir_iterate_block() spots this function, it'll pass the FID
* uniquifier through dtype
*/
-static int afs_lookup_one_filldir(struct dir_context *ctx, const char *name,
+static bool afs_lookup_one_filldir(struct dir_context *ctx, const char *name,
int nlen, loff_t fpos, u64 ino, unsigned dtype)
{
struct afs_lookup_one_cookie *cookie =
@@ -509,16 +511,16 @@ static int afs_lookup_one_filldir(struct dir_context *ctx, const char *name,
if (cookie->name.len != nlen ||
memcmp(cookie->name.name, name, nlen) != 0) {
- _leave(" = 0 [no]");
- return 0;
+ _leave(" = true [no]");
+ return true;
}
cookie->fid.vnode = ino;
cookie->fid.unique = dtype;
cookie->found = 1;
- _leave(" = -1 [found]");
- return -1;
+ _leave(" = false [found]");
+ return false;
}
/*
@@ -561,12 +563,12 @@ static int afs_do_lookup_one(struct inode *dir, struct dentry *dentry,
* - if afs_dir_iterate_block() spots this function, it'll pass the FID
* uniquifier through dtype
*/
-static int afs_lookup_filldir(struct dir_context *ctx, const char *name,
+static bool afs_lookup_filldir(struct dir_context *ctx, const char *name,
int nlen, loff_t fpos, u64 ino, unsigned dtype)
{
struct afs_lookup_cookie *cookie =
container_of(ctx, struct afs_lookup_cookie, ctx);
- int ret;
+ bool ret;
_enter("{%s,%u},%s,%u,,%llu,%u",
cookie->name.name, cookie->name.len, name, nlen,
@@ -588,11 +590,11 @@ static int afs_lookup_filldir(struct dir_context *ctx, const char *name,
cookie->fids[0].unique = dtype;
cookie->found = 1;
if (cookie->one_only)
- return -1;
+ return false;
}
- ret = cookie->nr_fids >= 50 ? -1 : 0;
- _leave(" = %d", ret);
+ ret = cookie->nr_fids < 50;
+ _leave(" = %d", (int)ret);
return ret;
}
diff --git a/fs/ecryptfs/file.c b/fs/ecryptfs/file.c
index b76a9853325e..79af7e199ae5 100644
--- a/fs/ecryptfs/file.c
+++ b/fs/ecryptfs/file.c
@@ -67,7 +67,7 @@ struct ecryptfs_getdents_callback {
};
/* Inspired by generic filldir in fs/readdir.c */
-static int
+static bool
ecryptfs_filldir(struct dir_context *ctx, const char *lower_name,
int lower_namelen, loff_t offset, u64 ino, unsigned int d_type)
{
@@ -76,6 +76,7 @@ ecryptfs_filldir(struct dir_context *ctx, const char *lower_name,
size_t name_size;
char *name;
int rc;
+ bool emit_success;
buf->filldir_called++;
rc = ecryptfs_decode_and_decrypt_filename(&name, &name_size,
@@ -86,7 +87,7 @@ ecryptfs_filldir(struct dir_context *ctx, const char *lower_name,
ecryptfs_printk(KERN_DEBUG,
"%s: Error attempting to decode and decrypt filename [%s]; rc = [%d]\n",
__func__, lower_name, rc);
- return rc;
+ return false;
}
/* Mask -EINVAL errors as these are most likely due a plaintext
@@ -95,16 +96,16 @@ ecryptfs_filldir(struct dir_context *ctx, const char *lower_name,
* the "lost+found" dentry in the root directory of an Ext4
* filesystem.
*/
- return 0;
+ return true;
}
buf->caller->pos = buf->ctx.pos;
- rc = !dir_emit(buf->caller, name, name_size, ino, d_type);
+ emit_success = dir_emit(buf->caller, name, name_size, ino, d_type);
kfree(name);
- if (!rc)
+ if (emit_success)
buf->entries_written++;
- return rc;
+ return emit_success;
}
/**
diff --git a/fs/exportfs/expfs.c b/fs/exportfs/expfs.c
index c69927bed4ef..9f159861fb24 100644
--- a/fs/exportfs/expfs.c
+++ b/fs/exportfs/expfs.c
@@ -247,21 +247,21 @@ struct getdents_callback {
* A rather strange filldir function to capture
* the name matching the specified inode number.
*/
-static int filldir_one(struct dir_context *ctx, const char *name, int len,
+static bool filldir_one(struct dir_context *ctx, const char *name, int len,
loff_t pos, u64 ino, unsigned int d_type)
{
struct getdents_callback *buf =
container_of(ctx, struct getdents_callback, ctx);
- int result = 0;
+ bool read_more = true;
buf->sequence++;
if (buf->ino == ino && len <= NAME_MAX) {
memcpy(buf->name, name, len);
buf->name[len] = '\0';
buf->found = 1;
- result = -1;
+ read_more = false;
}
- return result;
+ return read_more;
}
/**
diff --git a/fs/fat/dir.c b/fs/fat/dir.c
index 9d01db37183f..89d623ef5259 100644
--- a/fs/fat/dir.c
+++ b/fs/fat/dir.c
@@ -706,7 +706,7 @@ static int fat_readdir(struct file *file, struct dir_context *ctx)
}
#define FAT_IOCTL_FILLDIR_FUNC(func, dirent_type) \
-static int func(struct dir_context *ctx, const char *name, int name_len, \
+static bool func(struct dir_context *ctx, const char *name, int name_len, \
loff_t offset, u64 ino, unsigned int d_type) \
{ \
struct fat_ioctl_filldir_callback *buf = \
@@ -715,7 +715,7 @@ static int func(struct dir_context *ctx, const char *name, int name_len, \
struct dirent_type __user *d2 = d1 + 1; \
\
if (buf->result) \
- return -EINVAL; \
+ return false; \
buf->result++; \
\
if (name != NULL) { \
@@ -751,10 +751,10 @@ static int func(struct dir_context *ctx, const char *name, int name_len, \
put_user(short_len, &d1->d_reclen)) \
goto efault; \
} \
- return 0; \
+ return true; \
efault: \
buf->result = -EFAULT; \
- return -EFAULT; \
+ return false; \
}
FAT_IOCTL_FILLDIR_FUNC(fat_ioctl_filldir, __fat_dirent)
diff --git a/fs/gfs2/export.c b/fs/gfs2/export.c
index a332f3cd925e..20a619c14f60 100644
--- a/fs/gfs2/export.c
+++ b/fs/gfs2/export.c
@@ -69,7 +69,7 @@ struct get_name_filldir {
char *name;
};
-static int get_name_filldir(struct dir_context *ctx, const char *name,
+static bool get_name_filldir(struct dir_context *ctx, const char *name,
int length, loff_t offset, u64 inum,
unsigned int type)
{
@@ -77,12 +77,12 @@ static int get_name_filldir(struct dir_context *ctx, const char *name,
container_of(ctx, struct get_name_filldir, ctx);
if (inum != gnfd->inum.no_addr)
- return 0;
+ return true;
memcpy(gnfd->name, name, length);
gnfd->name[length] = 0;
- return 1;
+ return false;
}
static int gfs2_get_name(struct dentry *parent, char *name,
diff --git a/fs/nfsd/nfs4recover.c b/fs/nfsd/nfs4recover.c
index 5188f9f70c78..bcec773cf660 100644
--- a/fs/nfsd/nfs4recover.c
+++ b/fs/nfsd/nfs4recover.c
@@ -250,7 +250,7 @@ struct nfs4_dir_ctx {
struct list_head names;
};
-static int
+static bool
nfsd4_build_namelist(struct dir_context *__ctx, const char *name, int namlen,
loff_t offset, u64 ino, unsigned int d_type)
{
@@ -259,14 +259,14 @@ nfsd4_build_namelist(struct dir_context *__ctx, const char *name, int namlen,
struct name_list *entry;
if (namlen != HEXDIR_LEN - 1)
- return 0;
+ return true;
entry = kmalloc(sizeof(struct name_list), GFP_KERNEL);
if (entry == NULL)
- return -ENOMEM;
+ return false;
memcpy(entry->name, name, HEXDIR_LEN - 1);
entry->name[HEXDIR_LEN - 1] = '\0';
list_add(&entry->list, &ctx->names);
- return 0;
+ return true;
}
static int
diff --git a/fs/nfsd/vfs.c b/fs/nfsd/vfs.c
index 9824e32b2f23..4df9479f2cab 100644
--- a/fs/nfsd/vfs.c
+++ b/fs/nfsd/vfs.c
@@ -1831,7 +1831,7 @@ struct readdir_data {
int full;
};
-static int nfsd_buffered_filldir(struct dir_context *ctx, const char *name,
+static bool nfsd_buffered_filldir(struct dir_context *ctx, const char *name,
int namlen, loff_t offset, u64 ino,
unsigned int d_type)
{
@@ -1843,7 +1843,7 @@ static int nfsd_buffered_filldir(struct dir_context *ctx, const char *name,
reclen = ALIGN(sizeof(struct buffered_dirent) + namlen, sizeof(u64));
if (buf->used + reclen > PAGE_SIZE) {
buf->full = 1;
- return -EINVAL;
+ return false;
}
de->namlen = namlen;
@@ -1853,7 +1853,7 @@ static int nfsd_buffered_filldir(struct dir_context *ctx, const char *name,
memcpy(de->name, name, namlen);
buf->used += reclen;
- return 0;
+ return true;
}
static __be32 nfsd_buffered_readdir(struct file *file, nfsd_filldir_t func,
diff --git a/fs/ocfs2/dir.c b/fs/ocfs2/dir.c
index c121abbdfc7d..0eebf587e792 100644
--- a/fs/ocfs2/dir.c
+++ b/fs/ocfs2/dir.c
@@ -2060,7 +2060,7 @@ struct ocfs2_empty_dir_priv {
unsigned seen_other;
unsigned dx_dir;
};
-static int ocfs2_empty_dir_filldir(struct dir_context *ctx, const char *name,
+static bool ocfs2_empty_dir_filldir(struct dir_context *ctx, const char *name,
int name_len, loff_t pos, u64 ino,
unsigned type)
{
@@ -2080,7 +2080,7 @@ static int ocfs2_empty_dir_filldir(struct dir_context *ctx, const char *name,
*/
if (name_len == 1 && !strncmp(".", name, 1) && pos == 0) {
p->seen_dot = 1;
- return 0;
+ return true;
}
if (name_len == 2 && !strncmp("..", name, 2) &&
@@ -2088,13 +2088,13 @@ static int ocfs2_empty_dir_filldir(struct dir_context *ctx, const char *name,
p->seen_dot_dot = 1;
if (p->dx_dir && p->seen_dot)
- return 1;
+ return false;
- return 0;
+ return true;
}
p->seen_other = 1;
- return 1;
+ return false;
}
static int ocfs2_empty_dir_dx(struct inode *inode,
diff --git a/fs/ocfs2/journal.c b/fs/ocfs2/journal.c
index 46fd3ef2cf21..11b3c38d22e5 100644
--- a/fs/ocfs2/journal.c
+++ b/fs/ocfs2/journal.c
@@ -2036,7 +2036,7 @@ struct ocfs2_orphan_filldir_priv {
enum ocfs2_orphan_reco_type orphan_reco_type;
};
-static int ocfs2_orphan_filldir(struct dir_context *ctx, const char *name,
+static bool ocfs2_orphan_filldir(struct dir_context *ctx, const char *name,
int name_len, loff_t pos, u64 ino,
unsigned type)
{
@@ -2045,21 +2045,21 @@ static int ocfs2_orphan_filldir(struct dir_context *ctx, const char *name,
struct inode *iter;
if (name_len == 1 && !strncmp(".", name, 1))
- return 0;
+ return true;
if (name_len == 2 && !strncmp("..", name, 2))
- return 0;
+ return true;
/* do not include dio entry in case of orphan scan */
if ((p->orphan_reco_type == ORPHAN_NO_NEED_TRUNCATE) &&
(!strncmp(name, OCFS2_DIO_ORPHAN_PREFIX,
OCFS2_DIO_ORPHAN_PREFIX_LEN)))
- return 0;
+ return true;
/* Skip bad inodes so that recovery can continue */
iter = ocfs2_iget(p->osb, ino,
OCFS2_FI_FLAG_ORPHAN_RECOVERY, 0);
if (IS_ERR(iter))
- return 0;
+ return true;
if (!strncmp(name, OCFS2_DIO_ORPHAN_PREFIX,
OCFS2_DIO_ORPHAN_PREFIX_LEN))
@@ -2069,7 +2069,7 @@ static int ocfs2_orphan_filldir(struct dir_context *ctx, const char *name,
* happen concurrently with unlink/rename */
if (OCFS2_I(iter)->ip_next_orphan) {
iput(iter);
- return 0;
+ return true;
}
trace_ocfs2_orphan_filldir((unsigned long long)OCFS2_I(iter)->ip_blkno);
@@ -2078,7 +2078,7 @@ static int ocfs2_orphan_filldir(struct dir_context *ctx, const char *name,
OCFS2_I(iter)->ip_next_orphan = p->head;
p->head = iter;
- return 0;
+ return true;
}
static int ocfs2_queue_orphans(struct ocfs2_super *osb,
diff --git a/fs/overlayfs/readdir.c b/fs/overlayfs/readdir.c
index cc8303a806b4..e72b84a4b470 100644
--- a/fs/overlayfs/readdir.c
+++ b/fs/overlayfs/readdir.c
@@ -173,7 +173,7 @@ static struct ovl_cache_entry *ovl_cache_entry_new(struct ovl_readdir_data *rdd,
return p;
}
-static int ovl_cache_entry_add_rb(struct ovl_readdir_data *rdd,
+static bool ovl_cache_entry_add_rb(struct ovl_readdir_data *rdd,
const char *name, int len, u64 ino,
unsigned int d_type)
{
@@ -182,19 +182,19 @@ static int ovl_cache_entry_add_rb(struct ovl_readdir_data *rdd,
struct ovl_cache_entry *p;
if (ovl_cache_entry_find_link(name, len, &newp, &parent))
- return 0;
+ return true;
p = ovl_cache_entry_new(rdd, name, len, ino, d_type);
if (p == NULL) {
rdd->err = -ENOMEM;
- return -ENOMEM;
+ return false;
}
list_add_tail(&p->l_node, rdd->list);
rb_link_node(&p->node, parent, newp);
rb_insert_color(&p->node, rdd->root);
- return 0;
+ return true;
}
static int ovl_fill_lowest(struct ovl_readdir_data *rdd,
@@ -253,7 +253,7 @@ static void ovl_cache_put(struct ovl_dir_file *od, struct dentry *dentry)
}
}
-static int ovl_fill_merge(struct dir_context *ctx, const char *name,
+static bool ovl_fill_merge(struct dir_context *ctx, const char *name,
int namelen, loff_t offset, u64 ino,
unsigned int d_type)
{
@@ -526,7 +526,7 @@ static int ovl_cache_update_ino(struct path *path, struct ovl_cache_entry *p)
goto out;
}
-static int ovl_fill_plain(struct dir_context *ctx, const char *name,
+static bool ovl_fill_plain(struct dir_context *ctx, const char *name,
int namelen, loff_t offset, u64 ino,
unsigned int d_type)
{
@@ -538,11 +538,11 @@ static int ovl_fill_plain(struct dir_context *ctx, const char *name,
p = ovl_cache_entry_new(rdd, name, namelen, ino, d_type);
if (p == NULL) {
rdd->err = -ENOMEM;
- return -ENOMEM;
+ return false;
}
list_add_tail(&p->l_node, rdd->list);
- return 0;
+ return true;
}
static int ovl_dir_read_impure(struct path *path, struct list_head *list,
@@ -644,7 +644,7 @@ struct ovl_readdir_translate {
int xinobits;
};
-static int ovl_fill_real(struct dir_context *ctx, const char *name,
+static bool ovl_fill_real(struct dir_context *ctx, const char *name,
int namelen, loff_t offset, u64 ino,
unsigned int d_type)
{
@@ -980,7 +980,7 @@ void ovl_cleanup_whiteouts(struct dentry *upper, struct list_head *list)
inode_unlock(upper->d_inode);
}
-static int ovl_check_d_type(struct dir_context *ctx, const char *name,
+static bool ovl_check_d_type(struct dir_context *ctx, const char *name,
int namelen, loff_t offset, u64 ino,
unsigned int d_type)
{
@@ -989,12 +989,12 @@ static int ovl_check_d_type(struct dir_context *ctx, const char *name,
/* Even if d_type is not supported, DT_DIR is returned for . and .. */
if (!strncmp(name, ".", namelen) || !strncmp(name, "..", namelen))
- return 0;
+ return true;
if (d_type != DT_UNKNOWN)
rdd->d_type_supported = true;
- return 0;
+ return true;
}
/*
diff --git a/fs/readdir.c b/fs/readdir.c
index 58088510bb9c..83b552263a5a 100644
--- a/fs/readdir.c
+++ b/fs/readdir.c
@@ -108,7 +108,7 @@ struct readdir_callback {
int result;
};
-static int fillonedir(struct dir_context *ctx, const char *name, int namlen,
+static bool fillonedir(struct dir_context *ctx, const char *name, int namlen,
loff_t offset, u64 ino, unsigned int d_type)
{
struct readdir_callback *buf =
@@ -117,14 +117,14 @@ static int fillonedir(struct dir_context *ctx, const char *name, int namlen,
unsigned long d_ino;
if (buf->result)
- return -EINVAL;
+ return false;
buf->result = check_dirent_name(name, namlen);
if (unlikely(buf->result))
- return -EFSCORRUPTED;
+ return false;
d_ino = ino;
if (sizeof(d_ino) < sizeof(ino) && d_ino != ino) {
buf->result = -EOVERFLOW;
- return -EOVERFLOW;
+ return false;
}
buf->result++;
dirent = buf->dirent;
@@ -138,10 +138,10 @@ static int fillonedir(struct dir_context *ctx, const char *name, int namlen,
__copy_to_user(dirent->d_name, name, namlen) ||
__put_user(0, dirent->d_name + namlen))
goto efault;
- return 0;
+ return true;
efault:
buf->result = -EFAULT;
- return -EFAULT;
+ return false;
}
SYSCALL_DEFINE3(old_readdir, unsigned int, fd,
@@ -186,7 +186,7 @@ struct getdents_callback {
int error;
};
-static int filldir(struct dir_context *ctx, const char *name, int namlen,
+static bool filldir(struct dir_context *ctx, const char *name, int namlen,
loff_t offset, u64 ino, unsigned int d_type)
{
struct linux_dirent __user * dirent;
@@ -198,19 +198,19 @@ static int filldir(struct dir_context *ctx, const char *name, int namlen,
buf->error = check_dirent_name(name, namlen);
if (unlikely(buf->error))
- return -EFSCORRUPTED;
+ return false;
buf->error = -EINVAL; /* only used if we fail.. */
if (reclen > buf->count)
- return -EINVAL;
+ return false;
d_ino = ino;
if (sizeof(d_ino) < sizeof(ino) && d_ino != ino) {
buf->error = -EOVERFLOW;
- return -EOVERFLOW;
+ return false;
}
dirent = buf->previous;
if (dirent) {
if (signal_pending(current))
- return -EINTR;
+ return false;
if (__put_user(offset, &dirent->d_off))
goto efault;
}
@@ -229,10 +229,10 @@ static int filldir(struct dir_context *ctx, const char *name, int namlen,
dirent = (void __user *)dirent + reclen;
buf->current_dir = dirent;
buf->count -= reclen;
- return 0;
+ return true;
efault:
buf->error = -EFAULT;
- return -EFAULT;
+ return false;
}
SYSCALL_DEFINE3(getdents, unsigned int, fd,
@@ -276,7 +276,7 @@ struct getdents_callback64 {
int error;
};
-static int filldir64(struct dir_context *ctx, const char *name, int namlen,
+static bool filldir64(struct dir_context *ctx, const char *name, int namlen,
loff_t offset, u64 ino, unsigned int d_type)
{
struct linux_dirent64 __user *dirent;
@@ -287,14 +287,14 @@ static int filldir64(struct dir_context *ctx, const char *name, int namlen,
buf->error = check_dirent_name(name, namlen);
if (unlikely(buf->error))
- return -EFSCORRUPTED;
+ return false;
buf->error = -EINVAL; /* only used if we fail.. */
if (reclen > buf->count)
- return -EINVAL;
+ return false;
dirent = buf->previous;
if (dirent) {
if (signal_pending(current))
- return -EINTR;
+ return false;
if (__put_user(offset, &dirent->d_off))
goto efault;
}
@@ -315,10 +315,10 @@ static int filldir64(struct dir_context *ctx, const char *name, int namlen,
dirent = (void __user *)dirent + reclen;
buf->current_dir = dirent;
buf->count -= reclen;
- return 0;
+ return true;
efault:
buf->error = -EFAULT;
- return -EFAULT;
+ return false;
}
int ksys_getdents64(unsigned int fd, struct linux_dirent64 __user *dirent,
@@ -376,7 +376,7 @@ struct compat_readdir_callback {
int result;
};
-static int compat_fillonedir(struct dir_context *ctx, const char *name,
+static bool compat_fillonedir(struct dir_context *ctx, const char *name,
int namlen, loff_t offset, u64 ino,
unsigned int d_type)
{
@@ -389,11 +389,11 @@ static int compat_fillonedir(struct dir_context *ctx, const char *name,
return -EINVAL;
buf->result = check_dirent_name(name, namlen);
if (unlikely(buf->result))
- return -EFSCORRUPTED;
+ return false;
d_ino = ino;
if (sizeof(d_ino) < sizeof(ino) && d_ino != ino) {
buf->result = -EOVERFLOW;
- return -EOVERFLOW;
+ return false;
}
buf->result++;
dirent = buf->dirent;
@@ -407,10 +407,10 @@ static int compat_fillonedir(struct dir_context *ctx, const char *name,
__copy_to_user(dirent->d_name, name, namlen) ||
__put_user(0, dirent->d_name + namlen))
goto efault;
- return 0;
+ return true;
efault:
buf->result = -EFAULT;
- return -EFAULT;
+ return false;
}
COMPAT_SYSCALL_DEFINE3(old_readdir, unsigned int, fd,
@@ -449,8 +449,8 @@ struct compat_getdents_callback {
int error;
};
-static int compat_filldir(struct dir_context *ctx, const char *name, int namlen,
- loff_t offset, u64 ino, unsigned int d_type)
+static bool compat_filldir(struct dir_context *ctx, const char *name,
+ int namlen, loff_t offset, u64 ino, unsigned int d_type)
{
struct compat_linux_dirent __user * dirent;
struct compat_getdents_callback *buf =
@@ -461,19 +461,19 @@ static int compat_filldir(struct dir_context *ctx, const char *name, int namlen,
buf->error = check_dirent_name(name, namlen);
if (unlikely(buf->error))
- return -EFSCORRUPTED;
+ return false;
buf->error = -EINVAL; /* only used if we fail.. */
if (reclen > buf->count)
- return -EINVAL;
+ return false;
d_ino = ino;
if (sizeof(d_ino) < sizeof(ino) && d_ino != ino) {
buf->error = -EOVERFLOW;
- return -EOVERFLOW;
+ return false;
}
dirent = buf->previous;
if (dirent) {
if (signal_pending(current))
- return -EINTR;
+ return false;
if (__put_user(offset, &dirent->d_off))
goto efault;
}
@@ -492,10 +492,10 @@ static int compat_filldir(struct dir_context *ctx, const char *name, int namlen,
dirent = (void __user *)dirent + reclen;
buf->current_dir = dirent;
buf->count -= reclen;
- return 0;
+ return true;
efault:
buf->error = -EFAULT;
- return -EFAULT;
+ return false;
}
COMPAT_SYSCALL_DEFINE3(getdents, unsigned int, fd,
diff --git a/fs/reiserfs/xattr.c b/fs/reiserfs/xattr.c
index 32d8986c26fb..0d56b2a3adad 100644
--- a/fs/reiserfs/xattr.c
+++ b/fs/reiserfs/xattr.c
@@ -189,7 +189,7 @@ struct reiserfs_dentry_buf {
struct dentry *dentries[8];
};
-static int
+static bool
fill_with_dentries(struct dir_context *ctx, const char *name, int namelen,
loff_t offset, u64 ino, unsigned int d_type)
{
@@ -200,16 +200,16 @@ fill_with_dentries(struct dir_context *ctx, const char *name, int namelen,
WARN_ON_ONCE(!inode_is_locked(d_inode(dbuf->xadir)));
if (dbuf->count == ARRAY_SIZE(dbuf->dentries))
- return -ENOSPC;
+ return false;
if (name[0] == '.' && (namelen < 2 ||
(namelen == 2 && name[1] == '.')))
- return 0;
+ return true;
dentry = lookup_one_len(name, dbuf->xadir, namelen);
if (IS_ERR(dentry)) {
dbuf->err = PTR_ERR(dentry);
- return PTR_ERR(dentry);
+ return false;
} else if (d_really_is_negative(dentry)) {
/* A directory entry exists, but no file? */
reiserfs_error(dentry->d_sb, "xattr-20003",
@@ -218,11 +218,11 @@ fill_with_dentries(struct dir_context *ctx, const char *name, int namelen,
dentry, dbuf->xadir);
dput(dentry);
dbuf->err = -EIO;
- return -EIO;
+ return false;
}
dbuf->dentries[dbuf->count++] = dentry;
- return 0;
+ return true;
}
static void
@@ -780,7 +780,7 @@ struct listxattr_buf {
struct dentry *dentry;
};
-static int listxattr_filler(struct dir_context *ctx, const char *name,
+static bool listxattr_filler(struct dir_context *ctx, const char *name,
int namelen, loff_t offset, u64 ino,
unsigned int d_type)
{
@@ -796,19 +796,19 @@ static int listxattr_filler(struct dir_context *ctx, const char *name,
name);
if (!handler /* Unsupported xattr name */ ||
(handler->list && !handler->list(b->dentry)))
- return 0;
+ return true;
size = namelen + 1;
if (b->buf) {
if (b->pos + size > b->size) {
b->pos = -ERANGE;
- return -ERANGE;
+ return false;
}
memcpy(b->buf + b->pos, name, namelen);
b->buf[b->pos + namelen] = 0;
}
b->pos += size;
}
- return 0;
+ return true;
}
/*
diff --git a/fs/xfs/scrub/dir.c b/fs/xfs/scrub/dir.c
index cd3e4d768a18..e57069a299fa 100644
--- a/fs/xfs/scrub/dir.c
+++ b/fs/xfs/scrub/dir.c
@@ -100,7 +100,7 @@ xchk_dir_check_ftype(
* we check the inode number to make sure it's sane, then we check that
* we can look up this filename. Finally, we check the ftype.
*/
-STATIC int
+STATIC bool
xchk_dir_actor(
struct dir_context *dir_iter,
const char *name,
@@ -170,13 +170,13 @@ xchk_dir_actor(
goto out;
out:
/*
- * A negative error code returned here is supposed to cause the
+ * Returning false here causes the
* dir_emit caller (xfs_readdir) to abort the directory iteration
* and return zero to xchk_directory.
*/
if (error == 0 && sdc->sc->sm->sm_flags & XFS_SCRUB_OFLAG_CORRUPT)
- return -EFSCORRUPTED;
- return error;
+ return false;
+ return !error;
}
/* Scrub a directory btree record. */
diff --git a/fs/xfs/scrub/parent.c b/fs/xfs/scrub/parent.c
index 1c9d7c7f64f5..9b6d9d416039 100644
--- a/fs/xfs/scrub/parent.c
+++ b/fs/xfs/scrub/parent.c
@@ -45,7 +45,7 @@ struct xchk_parent_ctx {
};
/* Look for a single entry in a directory pointing to an inode. */
-STATIC int
+STATIC bool
xchk_parent_actor(
struct dir_context *dc,
const char *name,
@@ -59,7 +59,7 @@ xchk_parent_actor(
spc = container_of(dc, struct xchk_parent_ctx, dc);
if (spc->ino == ino)
spc->nlink++;
- return 0;
+ return true;
}
/* Count the number of dentries in the parent dir that point to this inode. */
diff --git a/include/linux/fs.h b/include/linux/fs.h
index e14329741e3a..1d6300c41d92 100644
--- a/include/linux/fs.h
+++ b/include/linux/fs.h
@@ -1720,9 +1720,11 @@ int fiemap_check_flags(struct fiemap_extent_info *fieinfo, u32 fs_flags);
* the kernel specify what kind of dirent layout it wants to have.
* This allows the kernel to read directories into kernel space or
* to have different dirent layouts depending on the binary type.
+ * Returns true if the provided entry has been consumed and the
+ * filldir function should be called again for the next entry.
*/
struct dir_context;
-typedef int (*filldir_t)(struct dir_context *, const char *, int, loff_t, u64,
+typedef bool (*filldir_t)(struct dir_context *, const char *, int, loff_t, u64,
unsigned);
struct dir_context {
@@ -3469,17 +3471,17 @@ static inline bool dir_emit(struct dir_context *ctx,
const char *name, int namelen,
u64 ino, unsigned type)
{
- return ctx->actor(ctx, name, namelen, ctx->pos, ino, type) == 0;
+ return ctx->actor(ctx, name, namelen, ctx->pos, ino, type);
}
static inline bool dir_emit_dot(struct file *file, struct dir_context *ctx)
{
return ctx->actor(ctx, ".", 1, ctx->pos,
- file->f_path.dentry->d_inode->i_ino, DT_DIR) == 0;
+ file->f_path.dentry->d_inode->i_ino, DT_DIR);
}
static inline bool dir_emit_dotdot(struct file *file, struct dir_context *ctx)
{
return ctx->actor(ctx, "..", 2, ctx->pos,
- parent_ino(file->f_path.dentry), DT_DIR) == 0;
+ parent_ino(file->f_path.dentry), DT_DIR);
}
static inline bool dir_emit_dots(struct file *file, struct dir_context *ctx)
{
--
2.20.1.321.g9e740568ce-goog
^ permalink raw reply related
* [PATCH v4 2/3] fs: don't let getdents return bogus names
From: Jann Horn @ 2019-01-18 16:14 UTC (permalink / raw)
To: Richard Henderson, Ivan Kokshaysky, Matt Turner, Alexander Viro,
linux-fsdevel, Arnd Bergmann, jannh
Cc: Eric W. Biederman, Theodore Ts'o, Andreas Dilger, linux-alpha,
linux-kernel, Dave Chinner, Pavel Machek, linux-arch, linux-api
In-Reply-To: <20190118161440.220134-1-jannh@google.com>
When you e.g. run `find` on a directory for which getdents returns
"filenames" that contain slashes, `find` passes those "filenames" back to
the kernel, which then interprets them as paths. That could conceivably
cause userspace to do something bad when accessing something like an
untrusted USB stick, but I'm not aware of any specific example.
Instead of returning bogus filenames to userspace, return -EUCLEAN.
Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
Cc: stable@vger.kernel.org
Signed-off-by: Jann Horn <jannh@google.com>
---
I ordered this fix before the refactoring one so that it can easily be
backported.
changed in v2:
- move bogus_dirent_name() out of the #ifdef (kbuild test robot)
changed in v3:
- change calling convention (Al Viro)
- comment fix
changed in v4:
- use EFSCORRUPTED instead of EUCLEAN (Dave Chinner)
arch/alpha/kernel/osf_sys.c | 4 ++++
fs/readdir.c | 35 +++++++++++++++++++++++++++++++++++
include/linux/fs.h | 2 ++
3 files changed, 41 insertions(+)
diff --git a/arch/alpha/kernel/osf_sys.c b/arch/alpha/kernel/osf_sys.c
index 792586038808..db1c2144d477 100644
--- a/arch/alpha/kernel/osf_sys.c
+++ b/arch/alpha/kernel/osf_sys.c
@@ -40,6 +40,7 @@
#include <linux/vfs.h>
#include <linux/rcupdate.h>
#include <linux/slab.h>
+#include <linux/fs.h>
#include <asm/fpu.h>
#include <asm/io.h>
@@ -117,6 +118,9 @@ osf_filldir(struct dir_context *ctx, const char *name, int namlen,
unsigned int reclen = ALIGN(NAME_OFFSET + namlen + 1, sizeof(u32));
unsigned int d_ino;
+ buf->error = check_dirent_name(name, namlen);
+ if (unlikely(buf->error))
+ return -EFSCORRUPTED;
buf->error = -EINVAL; /* only used if we fail */
if (reclen > buf->count)
return -EINVAL;
diff --git a/fs/readdir.c b/fs/readdir.c
index 2f6a4534e0df..58088510bb9c 100644
--- a/fs/readdir.c
+++ b/fs/readdir.c
@@ -64,6 +64,26 @@ int iterate_dir(struct file *file, struct dir_context *ctx)
}
EXPORT_SYMBOL(iterate_dir);
+/*
+ * Most filesystems don't filter out bogus directory entry names, and userspace
+ * can get very confused by such names. Behave as if a filesystem error had
+ * happened while reading directory entries.
+ */
+int check_dirent_name(const char *name, int namlen)
+{
+ if (namlen == 0) {
+ pr_err_once("%s: filesystem returned bogus empty name\n",
+ __func__);
+ return -EFSCORRUPTED;
+ }
+ if (memchr(name, '/', namlen)) {
+ pr_err_once("%s: filesystem returned bogus name '%*pEhp' (contains slash)\n",
+ __func__, namlen, name);
+ return -EFSCORRUPTED;
+ }
+ return 0;
+}
+
/*
* Traditional linux readdir() handling..
*
@@ -98,6 +118,9 @@ static int fillonedir(struct dir_context *ctx, const char *name, int namlen,
if (buf->result)
return -EINVAL;
+ buf->result = check_dirent_name(name, namlen);
+ if (unlikely(buf->result))
+ return -EFSCORRUPTED;
d_ino = ino;
if (sizeof(d_ino) < sizeof(ino) && d_ino != ino) {
buf->result = -EOVERFLOW;
@@ -173,6 +196,9 @@ static int filldir(struct dir_context *ctx, const char *name, int namlen,
int reclen = ALIGN(offsetof(struct linux_dirent, d_name) + namlen + 2,
sizeof(long));
+ buf->error = check_dirent_name(name, namlen);
+ if (unlikely(buf->error))
+ return -EFSCORRUPTED;
buf->error = -EINVAL; /* only used if we fail.. */
if (reclen > buf->count)
return -EINVAL;
@@ -259,6 +285,9 @@ static int filldir64(struct dir_context *ctx, const char *name, int namlen,
int reclen = ALIGN(offsetof(struct linux_dirent64, d_name) + namlen + 1,
sizeof(u64));
+ buf->error = check_dirent_name(name, namlen);
+ if (unlikely(buf->error))
+ return -EFSCORRUPTED;
buf->error = -EINVAL; /* only used if we fail.. */
if (reclen > buf->count)
return -EINVAL;
@@ -358,6 +387,9 @@ static int compat_fillonedir(struct dir_context *ctx, const char *name,
if (buf->result)
return -EINVAL;
+ buf->result = check_dirent_name(name, namlen);
+ if (unlikely(buf->result))
+ return -EFSCORRUPTED;
d_ino = ino;
if (sizeof(d_ino) < sizeof(ino) && d_ino != ino) {
buf->result = -EOVERFLOW;
@@ -427,6 +459,9 @@ static int compat_filldir(struct dir_context *ctx, const char *name, int namlen,
int reclen = ALIGN(offsetof(struct compat_linux_dirent, d_name) +
namlen + 2, sizeof(compat_long_t));
+ buf->error = check_dirent_name(name, namlen);
+ if (unlikely(buf->error))
+ return -EFSCORRUPTED;
buf->error = -EINVAL; /* only used if we fail.. */
if (reclen > buf->count)
return -EINVAL;
diff --git a/include/linux/fs.h b/include/linux/fs.h
index 811c77743dad..e14329741e3a 100644
--- a/include/linux/fs.h
+++ b/include/linux/fs.h
@@ -1730,6 +1730,8 @@ struct dir_context {
loff_t pos;
};
+int check_dirent_name(const char *name, int namlen);
+
struct block_device_operations;
/* These macros are for out of kernel modules to test that
--
2.20.1.321.g9e740568ce-goog
^ permalink raw reply related
* [PATCH v4 1/3] fs: hoist EFSCORRUPTED definition into uapi header
From: Jann Horn @ 2019-01-18 16:14 UTC (permalink / raw)
To: Richard Henderson, Ivan Kokshaysky, Matt Turner, Alexander Viro,
linux-fsdevel, Arnd Bergmann, jannh
Cc: Eric W. Biederman, Theodore Ts'o, Andreas Dilger, linux-alpha,
linux-kernel, Dave Chinner, Pavel Machek, linux-arch, linux-api
Multiple filesystems can already return EFSCORRUPTED errors to userspace;
however, so far, definitions of EFSCORRUPTED were in filesystem-private
headers.
I wanted to use EUCLEAN to indicate data corruption in the VFS layer;
Dave Chinner says that I should instead hoist the definitions of
EFSCORRUPTED into the UAPI header and then use EFSCORRUPTED.
This patch is marked for stable backport because it is a prerequisite for
the following patch.
Cc: stable@vger.kernel.org
Suggested-by: Dave Chinner <david@fromorbit.com>
Signed-off-by: Jann Horn <jannh@google.com>
---
fs/ext2/ext2.h | 1 -
fs/ext4/ext4.h | 1 -
fs/xfs/xfs_linux.h | 1 -
include/linux/jbd2.h | 1 -
include/uapi/asm-generic/errno.h | 1 +
5 files changed, 1 insertion(+), 4 deletions(-)
diff --git a/fs/ext2/ext2.h b/fs/ext2/ext2.h
index e770cd100a6a..7fafc19e5aa0 100644
--- a/fs/ext2/ext2.h
+++ b/fs/ext2/ext2.h
@@ -369,7 +369,6 @@ struct ext2_inode {
*/
#define EXT2_VALID_FS 0x0001 /* Unmounted cleanly */
#define EXT2_ERROR_FS 0x0002 /* Errors detected */
-#define EFSCORRUPTED EUCLEAN /* Filesystem is corrupted */
/*
* Mount flags
diff --git a/fs/ext4/ext4.h b/fs/ext4/ext4.h
index 185a05d3257e..9397e97fc15b 100644
--- a/fs/ext4/ext4.h
+++ b/fs/ext4/ext4.h
@@ -3247,6 +3247,5 @@ extern const struct iomap_ops ext4_iomap_ops;
#endif /* __KERNEL__ */
#define EFSBADCRC EBADMSG /* Bad CRC detected */
-#define EFSCORRUPTED EUCLEAN /* Filesystem is corrupted */
#endif /* _EXT4_H */
diff --git a/fs/xfs/xfs_linux.h b/fs/xfs/xfs_linux.h
index edbd5a210df2..36e5c6549f15 100644
--- a/fs/xfs/xfs_linux.h
+++ b/fs/xfs/xfs_linux.h
@@ -125,7 +125,6 @@ typedef __u32 xfs_nlink_t;
#define ENOATTR ENODATA /* Attribute not found */
#define EWRONGFS EINVAL /* Mount with wrong filesystem type */
-#define EFSCORRUPTED EUCLEAN /* Filesystem is corrupted */
#define EFSBADCRC EBADMSG /* Bad CRC detected */
#define SYNCHRONIZE() barrier()
diff --git a/include/linux/jbd2.h b/include/linux/jbd2.h
index 0f919d5fe84f..1d0da9c78216 100644
--- a/include/linux/jbd2.h
+++ b/include/linux/jbd2.h
@@ -1640,6 +1640,5 @@ static inline tid_t jbd2_get_latest_transaction(journal_t *journal)
#endif /* __KERNEL__ */
#define EFSBADCRC EBADMSG /* Bad CRC detected */
-#define EFSCORRUPTED EUCLEAN /* Filesystem is corrupted */
#endif /* _LINUX_JBD2_H */
diff --git a/include/uapi/asm-generic/errno.h b/include/uapi/asm-generic/errno.h
index cf9c51ac49f9..5ddebc1bf951 100644
--- a/include/uapi/asm-generic/errno.h
+++ b/include/uapi/asm-generic/errno.h
@@ -98,6 +98,7 @@
#define EINPROGRESS 115 /* Operation now in progress */
#define ESTALE 116 /* Stale file handle */
#define EUCLEAN 117 /* Structure needs cleaning */
+#define EFSCORRUPTED EUCLEAN /* Filesystem is corrupted */
#define ENOTNAM 118 /* Not a XENIX named type file */
#define ENAVAIL 119 /* No XENIX semaphores available */
#define EISNAM 120 /* Is a named type file */
--
2.20.1.321.g9e740568ce-goog
^ permalink raw reply related
* Re: [PATCH] mm/mincore: allow for making sys_mincore() privileged
From: Tejun Heo @ 2019-01-18 14:23 UTC (permalink / raw)
To: Linus Torvalds
Cc: Jiri Kosina, Masatake YAMATO, Andrew Morton, Greg KH,
Peter Zijlstra, Michal Hocko, linux-mm, Linux List Kernel Mailing,
linux-api
In-Reply-To: <CAHk-=wgrSKyN23yp-npq6+J-4pGqbzxb3mJ183PryjHw7PWDyA@mail.gmail.com>
Hello, Linus.
On Sat, Jan 05, 2019 at 01:54:03PM -0800, Linus Torvalds wrote:
> And the first hit is 'fincore', which probably nobody cares about
> anyway, but it does
>
> fd = open (name, O_RDONLY)
> ..
> mmap(window, len, PROT_NONE, MAP_PRIVATE, ..
So, folks here have been using fincore(1) for diagnostic purposes and
are also looking to expand on it to investigate per-cgroup cache
usages (mmap -> mincore -> /proc/self/pagemap -> /proc/kpagecgroup ->
cgroup path).
These are all root-only usages to find out what's going on with the
whole page cache. We aren't attached to doing things this particular
way but it'd suck if there's no way.
Thanks.
--
tejun
^ permalink raw reply
* [RESEND PATCH v2 1/1] prctl: add PR_{GET,SET}_KILL_DESCENDANTS_ON_EXIT
From: Jürg Billeter @ 2019-01-18 13:11 UTC (permalink / raw)
To: Andrew Morton
Cc: Oleg Nesterov, Thomas Gleixner, Eric Biederman, Kees Cook,
Andy Lutomirski, linux-api, linux-kernel, Jürg Billeter
In-Reply-To: <20190118131130.42209-1-j@bitron.ch>
This introduces a new thread group flag that can be set by calling
prctl(PR_SET_KILL_DESCENDANTS_ON_EXIT, 1, 0, 0, 0)
When a thread group exits with this flag set, it will send SIGKILL to
all descendant processes. This can be used to prevent stray child
processes.
This flag is cleared on privilege gaining execve(2) to ensure an
unprivileged process cannot get a privileged process to send SIGKILL.
Descendants that are orphaned and reparented to an ancestor of the
current process before the current process exits, will not be killed.
PR_SET_CHILD_SUBREAPER can be used to contain orphaned processes.
If a descendant gained privileges, the current process may not be
allowed to kill it, and the descendant process will survive.
PR_SET_NO_NEW_PRIVS can be used to prevent descendant processes from
gaining privileges.
Suggested-by: Oleg Nesterov <oleg@redhat.com>
Signed-off-by: Jürg Billeter <j@bitron.ch>
Reviewed-by: Oleg Nesterov <oleg@redhat.com>
---
fs/exec.c | 6 ++++++
include/linux/sched/signal.h | 3 +++
include/uapi/linux/prctl.h | 4 ++++
kernel/exit.c | 12 ++++++++++++
kernel/sys.c | 11 +++++++++++
security/apparmor/lsm.c | 1 +
security/selinux/hooks.c | 3 +++
7 files changed, 40 insertions(+)
diff --git a/fs/exec.c b/fs/exec.c
index fb72d36f7823..bbb5a0718223 100644
--- a/fs/exec.c
+++ b/fs/exec.c
@@ -1342,6 +1342,12 @@ void setup_new_exec(struct linux_binprm * bprm)
/* Make sure parent cannot signal privileged process. */
current->pdeath_signal = 0;
+ /*
+ * Do not send SIGKILL from privileged process as it may
+ * have been requested by an unprivileged process.
+ */
+ current->signal->kill_descendants_on_exit = false;
+
/*
* For secureexec, reset the stack limit to sane default to
* avoid bad behavior from the prior rlimits. This has to
diff --git a/include/linux/sched/signal.h b/include/linux/sched/signal.h
index 13789d10a50e..2acf481951f6 100644
--- a/include/linux/sched/signal.h
+++ b/include/linux/sched/signal.h
@@ -124,6 +124,9 @@ struct signal_struct {
unsigned int is_child_subreaper:1;
unsigned int has_child_subreaper:1;
+ /* Send SIGKILL to descendant processes on exit */
+ bool kill_descendants_on_exit;
+
#ifdef CONFIG_POSIX_TIMERS
/* POSIX.1b Interval Timers */
diff --git a/include/uapi/linux/prctl.h b/include/uapi/linux/prctl.h
index b4875a93363a..d5483ca63c2d 100644
--- a/include/uapi/linux/prctl.h
+++ b/include/uapi/linux/prctl.h
@@ -198,6 +198,10 @@ struct prctl_mm_map {
# define PR_CAP_AMBIENT_LOWER 3
# define PR_CAP_AMBIENT_CLEAR_ALL 4
+/* Send SIGKILL to descendant processes on exit */
+#define PR_SET_KILL_DESCENDANTS_ON_EXIT 48
+#define PR_GET_KILL_DESCENDANTS_ON_EXIT 49
+
/* arm64 Scalable Vector Extension controls */
/* Flag values must be kept in sync with ptrace NT_ARM_SVE interface */
#define PR_SVE_SET_VL 50 /* set task vector length */
diff --git a/kernel/exit.c b/kernel/exit.c
index 2d14979577ee..93a812c1b670 100644
--- a/kernel/exit.c
+++ b/kernel/exit.c
@@ -694,6 +694,15 @@ static void forget_original_parent(struct task_struct *father,
list_splice_tail_init(&father->children, &reaper->children);
}
+static int kill_descendant_visitor(struct task_struct *p, void *data)
+{
+ /* This may fail, e.g., when a descendant process gained privileges. */
+ group_send_sig_info(SIGKILL, SEND_SIG_NOINFO, p, PIDTYPE_TGID);
+
+ /* Always continue walking the process tree. */
+ return 1;
+}
+
/*
* Send signals to all our closest relatives so that they know
* to properly mourn us..
@@ -704,6 +713,9 @@ static void exit_notify(struct task_struct *tsk, int group_dead)
struct task_struct *p, *n;
LIST_HEAD(dead);
+ if (group_dead && tsk->signal->kill_descendants_on_exit)
+ walk_process_tree(tsk, kill_descendant_visitor, NULL);
+
write_lock_irq(&tasklist_lock);
forget_original_parent(tsk, &dead);
diff --git a/kernel/sys.c b/kernel/sys.c
index f7eb62eceb24..f6dba0ba9b77 100644
--- a/kernel/sys.c
+++ b/kernel/sys.c
@@ -2485,6 +2485,17 @@ SYSCALL_DEFINE5(prctl, int, option, unsigned long, arg2, unsigned long, arg3,
return -EINVAL;
error = PAC_RESET_KEYS(me, arg2);
break;
+ case PR_SET_KILL_DESCENDANTS_ON_EXIT:
+ if (arg3 || arg4 || arg5)
+ return -EINVAL;
+ me->signal->kill_descendants_on_exit = !!arg2;
+ break;
+ case PR_GET_KILL_DESCENDANTS_ON_EXIT:
+ if (arg3 || arg4 || arg5)
+ return -EINVAL;
+ error = put_user(me->signal->kill_descendants_on_exit,
+ (int __user *)arg2);
+ break;
default:
error = -EINVAL;
break;
diff --git a/security/apparmor/lsm.c b/security/apparmor/lsm.c
index 2c010874329f..b09c6e0a25ff 100644
--- a/security/apparmor/lsm.c
+++ b/security/apparmor/lsm.c
@@ -698,6 +698,7 @@ static void apparmor_bprm_committing_creds(struct linux_binprm *bprm)
aa_inherit_files(bprm->cred, current->files);
current->pdeath_signal = 0;
+ current->signal->kill_descendants_on_exit = false;
/* reset soft limits and set hard limits for the new label */
__aa_transition_rlimits(label, new_label);
diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
index f0e36c3492ba..3bac775634c8 100644
--- a/security/selinux/hooks.c
+++ b/security/selinux/hooks.c
@@ -2510,6 +2510,9 @@ static void selinux_bprm_committing_creds(struct linux_binprm *bprm)
/* Always clear parent death signal on SID transitions. */
current->pdeath_signal = 0;
+ /* Disable SIGKILL requested from before the SID transition. */
+ current->signal->kill_descendants_on_exit = false;
+
/* Check whether the new SID can inherit resource limits from the old
* SID. If not, reset all soft limits to the lower of the current
* task's hard limit and the init task's soft limit.
--
2.20.1
^ permalink raw reply related
* [RESEND PATCH v2 0/1] Add prctl to kill descendants on exit
From: Jürg Billeter @ 2019-01-18 13:11 UTC (permalink / raw)
To: Andrew Morton
Cc: Oleg Nesterov, Thomas Gleixner, Eric Biederman, Kees Cook,
Andy Lutomirski, linux-api, linux-kernel, Jürg Billeter
In-Reply-To: <20181127225408.7553-1-j@bitron.ch>
This patch adds a new prctl to kill all descendant processes on exit.
See commit message for details of the prctl.
This is a replacement of PR_SET_PDEATHSIG_PROC I proposed last year [1].
In the following discussion, Oleg suggested this approach.
The motivation for this is to provide a lightweight mechanism to prevent
stray processes. There is also a related Bugzilla entry [2].
PID namespaces can also be used to prevent stray processes, of course.
However, they are not quite as lightweight as they typically also
require a new mount namespace to be able to mount a new /proc. And they
require CAP_SYS_ADMIN. User namespaces can help to gain CAP_SYS_ADMIN,
however, that further increases the overhead and the other effects of
the user namespace may not be desired.
PID 1 in PID namespaces also exhibits non-standard signal behavior
(SIGNAL_UNKILLABLE) [3].
Changes in v2:
- Use bool instead of bitfield to avoid race with
PR_SET_CHILD_SUBREAPER
[1] https://lkml.kernel.org/lkml/20170929123058.48924-1-j@bitron.ch/
[2] https://bugzilla.kernel.org/show_bug.cgi?id=43300
[3] https://lkml.kernel.org/lkml/20180803144021.56920-2-j@bitron.ch/
Jürg Billeter (1):
prctl: add PR_{GET,SET}_KILL_DESCENDANTS_ON_EXIT
fs/exec.c | 6 ++++++
include/linux/sched/signal.h | 3 +++
include/uapi/linux/prctl.h | 4 ++++
kernel/exit.c | 12 ++++++++++++
kernel/sys.c | 11 +++++++++++
security/apparmor/lsm.c | 1 +
security/selinux/hooks.c | 3 +++
7 files changed, 40 insertions(+)
--
2.20.1
^ permalink raw reply
* Re: vt: Improve CSI parsing
From: Greg Kroah-Hartman @ 2019-01-18 12:59 UTC (permalink / raw)
To: Martin Hostettler
Cc: Jiri Slaby, linux-kernel, linux-api, Nicolas Pitre, Adam Borowski,
Egmont Koblinger
In-Reply-To: <20181215143423.4556-1-textshell@uchuujin.de>
On Sat, Dec 15, 2018 at 03:34:19PM +0100, Martin Hostettler wrote:
> This patch series improves parsing of csi sequences to be more compliant
> with current practice.
>
> ECMA-64 defines the format of CSI sequences which allow more characters
> than what the vt parser currently accepts. More importantly many of
> these characters are used in sequences that more capable terminal
> terminal implementations use.
>
> Adjust the parsing of CSI sequences to match xterm* by ignoring all
> unknown sequences of the form
> (ESC [)|CSI [\x20-\x3f]*[\x40-\x7e]
>
> This avoids printing unwanted characters when application send valid
> sequences not supported by linux either while querying the terminal for
> it's identity or when applications print sequences without knowing what
> terminal implementation they are connected to (e.g. when connected over
> serial lines, android's adb, simple tcp connects, etc)
>
> * and other common terminals
Thanks for these, now queued up.
greg k-h
^ permalink raw reply
* Re: [PATCH] mm/mincore: allow for making sys_mincore() privileged
From: Linus Torvalds @ 2019-01-18 4:54 UTC (permalink / raw)
To: Matthew Wilcox
Cc: Jiri Kosina, Dominique Martinet, Andy Lutomirski, Josh Snyder,
Dave Chinner, Jann Horn, Andrew Morton, Greg KH, Peter Zijlstra,
Michal Hocko, Linux-MM, kernel list, Linux API
In-Reply-To: <CAHk-=wjciBwJo5JHcvUO+JAC13TUME1PH=ftsaNt+0RC-3PCSw@mail.gmail.com>
On Thu, Jan 17, 2019 at 4:51 PM Linus Torvalds
<torvalds@linux-foundation.org> wrote:
>
> On Thu, Jan 17, 2019 at 9:37 AM Matthew Wilcox <willy@infradead.org> wrote:
> >
> > Your patch 3/3 just removes the test. Am I right in thinking that it
> > doesn't need to be *moved* because the existing test after !PageUptodate
> > catches it?
>
> That's the _hope_.
>
> That's the simplest patch I can come up with as a potential solution.
> But it's possible that there's some nasty performance regression
> because somebody really relies on not even triggering read-ahead, and
> we might need to do some totally different thing.
Oh, and somebody should probably check that there isn't some simple
way to just avoid that readahead code entirely.
In particular, right now we skip readahead for at least these cases:
/* no read-ahead */
if (!ra->ra_pages)
return;
if (blk_cgroup_congested())
return;
and I don't think we need to worry about the cgroup congestion case -
if the attack has to also congest its cgroup with IO, I think they
have bigger problems.
And I think 'ra_pages' can be zero only in the presence of IO errors,
but I might be wrong. It would be good if somebody double-checks that.
Linus
^ permalink raw reply
* Re: [PATCH] mm/mincore: allow for making sys_mincore() privileged
From: Linus Torvalds @ 2019-01-18 4:49 UTC (permalink / raw)
To: Vlastimil Babka
Cc: Josh Snyder, Dominique Martinet, Andy Lutomirski, Dave Chinner,
Jiri Kosina, Matthew Wilcox, Jann Horn, Andrew Morton, Greg KH,
Peter Zijlstra, Michal Hocko, Linux-MM, kernel list, Linux API
In-Reply-To: <a056deb7-9c11-612e-2b3a-6482acca4ff6@suse.cz>
On Fri, Jan 18, 2019 at 9:45 AM Vlastimil Babka <vbabka@suse.cz> wrote:
>
> Or maybe we could resort to the 5.0-rc1 page table check (that is now being
> reverted) but only in cases when we are not allowed the page cache residency
> check? Or would that be needlessly complicated?
I think it would be good fallback semantics, but I'm not sure it's
worth it. Have you tried writing a patch for it? I don't think you'd
want to do the check *when* you find a hole, so you'd have to do it
upfront and then pass the cached data down with the private pointer
(or have a separate "struct mm_walk" structure, perhaps?
So I suspect we're better off with the patch we have. But if somebody
*wants* to try to do that fancier patch, and it doesn't look
horrendous, I think it might be the "quality" solution.
Linus
^ permalink raw reply
* Re: [PATCH] mm/mincore: allow for making sys_mincore() privileged
From: Vlastimil Babka @ 2019-01-17 21:45 UTC (permalink / raw)
To: Josh Snyder, Dominique Martinet
Cc: Linus Torvalds, Andy Lutomirski, Dave Chinner, Jiri Kosina,
Matthew Wilcox, Jann Horn, Andrew Morton, Greg KH, Peter Zijlstra,
Michal Hocko, Linux-MM, kernel list, Linux API
In-Reply-To: <CA+t-nXTfdo07EBvVo+mu8SRhrVyB=mEPLDQikHfpJue1jALJtQ@mail.gmail.com>
On 1/16/2019 8:52 AM, Josh Snyder wrote:
> On Tue, Jan 15, 2019 at 10:34 PM Dominique Martinet <asmadeus@codewreck.org>
> wrote:
>>
>> There is a difference with your previous patch though, that used to list no
>> page in core when it didn't know; this patch lists pages as in core when it
>> refuses to tell. I don't think that's very important, though.
I've argued previously that reporting false positives (as your patch does)
should be better, otherwise there might be somebody trying to fault in their
pages in a loop until mincore reports positive, which would become an endless
loop. So agreed with your change.
Or maybe we could resort to the 5.0-rc1 page table check (that is now being
reverted) but only in cases when we are not allowed the page cache residency
check? Or would that be needlessly complicated? And it would be able to leak if
a page was evicted from the page cache...
> Is there a reason not to return -EPERM in this case?
That would definitely break somebody.
>>
>> If anything, the 0400 user-owner file might be a problem in some edge
>> case (e.g. if you're preloading git directories, many objects are 0444);
>> should we *also* check ownership?...
>
> Yes, this seems valuable. Some databases with immutable files (e.g. git, as
> you've mentioned) conceivably operate this way.
>
> Josh
>
^ permalink raw reply
* Re: [PATCH] mm/mincore: allow for making sys_mincore() privileged
From: Dave Chinner @ 2019-01-17 21:06 UTC (permalink / raw)
To: Jiri Kosina
Cc: Linus Torvalds, Matthew Wilcox, Jann Horn, Andrew Morton, Greg KH,
Peter Zijlstra, Michal Hocko, Linux-MM, kernel list, Linux API
In-Reply-To: <nycvar.YFH.7.76.1901170917490.6626@cbobk.fhfr.pm>
On Thu, Jan 17, 2019 at 09:18:41AM +0100, Jiri Kosina wrote:
> On Thu, 17 Jan 2019, Dave Chinner wrote:
>
> > > > commit e837eac23662afae603aaaef7c94bc839c1b8f67
> > > > Author: Steve Lord <lord@sgi.com>
> > > > Date: Mon Mar 5 16:47:52 2001 +0000
> > > >
> > > > Add bounds checking for direct I/O, do the cache invalidation for
> > > > data coherency on direct I/O.
> > >
> > > Out of curiosity, which repository is this from please? Even google
> > > doesn't seem to know about this SHA.
> >
> > because oss.sgi.com is no longer with us, it's fallen out of all the
> > search engines. It was from the "archive/xfs-import.git" tree on
> > oss.sgi.com:
> >
> > https://web.archive.org/web/20120326044237/http://oss.sgi.com:80/cgi-bin/gitweb.cgi
> >
> > but archive.org doesn't have a copy of the git tree. It contained
> > the XFS history right back to the first Irix commit in 1993. Some of
> > us still have copies of it sitting around....
>
> For cases like this, would it be worth pushing it to git.kernel.org as an
> frozen historical reference archive?
I'm not sure we should be putting code from Irix on kernel.org. I
uploaded a copy to github a few months ago so XFS devs could easily
reference relevant commits in email. The reasons we decided not to
upload it to kernel.org should be clear from the readme...
https://github.com/dchinner/xfs-history
Cheers,
Dave.
--
Dave Chinner
david@fromorbit.com
^ permalink raw reply
* Re: [PATCH 2/5] ipc: introduce ksys_ipc()/compat_ksys_ipc() for s390
From: Heiko Carstens @ 2019-01-17 20:13 UTC (permalink / raw)
To: Arnd Bergmann
Cc: linux-s390, Martin Schwidefsky, Linux Kernel Mailing List,
y2038 Mailman List, Dominik Brodowski, Mark Rutland, Linux API
In-Reply-To: <CAK8P3a1u0OJKPnuBHVy4WSFy6T04kfNw5BwkpsLmkEU4KQD5eQ@mail.gmail.com>
On Thu, Jan 17, 2019 at 05:29:55PM +0100, Arnd Bergmann wrote:
> On Thu, Jan 17, 2019 at 2:29 PM Heiko Carstens
> > SYSCALL_DEFINE1(s390_personality, unsigned int, personality)
> > {
> > diff --git a/kernel/sys_ni.c b/kernel/sys_ni.c
> > index ab9d0e3c6d50..ad016a7db0ea 100644
> > --- a/kernel/sys_ni.c
> > +++ b/kernel/sys_ni.c
> > @@ -366,7 +366,7 @@ COND_SYSCALL(kexec_file_load);
> > /* s390 */
> > COND_SYSCALL(s390_pci_mmio_read);
> > COND_SYSCALL(s390_pci_mmio_write);
> > -COND_SYSCALL_COMPAT(s390_ipc);
> > +COND_SYSCALL(s390_ipc);
> >
> > /* powerpc */
> > COND_SYSCALL(rtas);
>
> I think you need to keep the
>
> COND_SYSCALL_COMPAT(s390_ipc);
>
> here, otherwise it still fails with CONFIG_SYSV_IPC=n, CONFIG_COMPAT=y, right?
Yes, you're right.
^ permalink raw reply
* Re: [PATCH 2/5] ipc: introduce ksys_ipc()/compat_ksys_ipc() for s390
From: Arnd Bergmann @ 2019-01-17 16:29 UTC (permalink / raw)
To: Heiko Carstens
Cc: linux-s390, Martin Schwidefsky, Linux Kernel Mailing List,
y2038 Mailman List, Dominik Brodowski, Mark Rutland, Linux API
In-Reply-To: <20190117132906.GB18351@osiris>
On Thu, Jan 17, 2019 at 2:29 PM Heiko Carstens
<heiko.carstens@de.ibm.com> wrote:
> diff --git a/arch/s390/kernel/sys_s390.c b/arch/s390/kernel/sys_s390.c
> index 6aa8fe00b39e..fd0cbbed4d9f 100644
> --- a/arch/s390/kernel/sys_s390.c
> +++ b/arch/s390/kernel/sys_s390.c
> @@ -58,6 +58,7 @@ SYSCALL_DEFINE1(mmap2, struct s390_mmap_arg_struct __user *, arg)
> return error;
> }
>
> +#ifdef CONFIG_SYSVIPC
> /*
> * sys_ipc() is the de-multiplexer for the SysV IPC calls.
> */
> @@ -76,6 +77,7 @@ SYSCALL_DEFINE5(s390_ipc, uint, call, int, first, unsigned long, second,
> */
> return ksys_ipc(call, first, second, third, ptr, third);
> }
> +#endif /* CONFIG_SYSVIPC */
Ack.
> SYSCALL_DEFINE1(s390_personality, unsigned int, personality)
> {
> diff --git a/kernel/sys_ni.c b/kernel/sys_ni.c
> index ab9d0e3c6d50..ad016a7db0ea 100644
> --- a/kernel/sys_ni.c
> +++ b/kernel/sys_ni.c
> @@ -366,7 +366,7 @@ COND_SYSCALL(kexec_file_load);
> /* s390 */
> COND_SYSCALL(s390_pci_mmio_read);
> COND_SYSCALL(s390_pci_mmio_write);
> -COND_SYSCALL_COMPAT(s390_ipc);
> +COND_SYSCALL(s390_ipc);
>
> /* powerpc */
> COND_SYSCALL(rtas);
I think you need to keep the
COND_SYSCALL_COMPAT(s390_ipc);
here, otherwise it still fails with CONFIG_SYSV_IPC=n, CONFIG_COMPAT=y, right?
Arnd
^ permalink raw reply
* Re: [PATCH 2/5] ipc: introduce ksys_ipc()/compat_ksys_ipc() for s390
From: Heiko Carstens @ 2019-01-17 13:29 UTC (permalink / raw)
To: Arnd Bergmann
Cc: linux-s390, Martin Schwidefsky, linux-kernel, y2038,
Dominik Brodowski, Mark Rutland, linux-api
In-Reply-To: <20190116131527.2071570-3-arnd@arndb.de>
On Wed, Jan 16, 2019 at 02:15:20PM +0100, Arnd Bergmann wrote:
> The sys_ipc() and compat_ksys_ipc() functions are meant to only
> be used from the system call table, not called by another function.
>
> Introduce ksys_*() interfaces for this purpose, as we have done
> for many other system calls.
>
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> ---
> arch/s390/kernel/compat_linux.c | 2 +-
> arch/s390/kernel/sys_s390.c | 2 +-
> include/linux/syscalls.h | 4 ++++
> ipc/syscall.c | 20 ++++++++++++++++----
> 4 files changed, 22 insertions(+), 6 deletions(-)
The patch below is needed as compile fix (allnoconfig).
I will add this to your patch, no resend needed.
diff --git a/arch/s390/kernel/sys_s390.c b/arch/s390/kernel/sys_s390.c
index 6aa8fe00b39e..fd0cbbed4d9f 100644
--- a/arch/s390/kernel/sys_s390.c
+++ b/arch/s390/kernel/sys_s390.c
@@ -58,6 +58,7 @@ SYSCALL_DEFINE1(mmap2, struct s390_mmap_arg_struct __user *, arg)
return error;
}
+#ifdef CONFIG_SYSVIPC
/*
* sys_ipc() is the de-multiplexer for the SysV IPC calls.
*/
@@ -76,6 +77,7 @@ SYSCALL_DEFINE5(s390_ipc, uint, call, int, first, unsigned long, second,
*/
return ksys_ipc(call, first, second, third, ptr, third);
}
+#endif /* CONFIG_SYSVIPC */
SYSCALL_DEFINE1(s390_personality, unsigned int, personality)
{
diff --git a/kernel/sys_ni.c b/kernel/sys_ni.c
index ab9d0e3c6d50..ad016a7db0ea 100644
--- a/kernel/sys_ni.c
+++ b/kernel/sys_ni.c
@@ -366,7 +366,7 @@ COND_SYSCALL(kexec_file_load);
/* s390 */
COND_SYSCALL(s390_pci_mmio_read);
COND_SYSCALL(s390_pci_mmio_write);
-COND_SYSCALL_COMPAT(s390_ipc);
+COND_SYSCALL(s390_ipc);
/* powerpc */
COND_SYSCALL(rtas);
^ permalink raw reply related
* Re: [PATCH] mm/mincore: allow for making sys_mincore() privileged
From: Cyril Hrubis @ 2019-01-17 9:52 UTC (permalink / raw)
To: Jiri Kosina
Cc: Matthew Wilcox, Linus Torvalds, Dominique Martinet,
Andy Lutomirski, Josh Snyder, Dave Chinner, Jann Horn,
Andrew Morton, Greg KH, Peter Zijlstra, Michal Hocko, Linux-MM,
kernel list, Linux API
In-Reply-To: <nycvar.YFH.7.76.1901162238310.6626@cbobk.fhfr.pm>
Hi!
> > Your patch 3/3 just removes the test. Am I right in thinking that it
> > doesn't need to be *moved* because the existing test after !PageUptodate
> > catches it?
>
> Exactly. It just initiates read-ahead for IOCB_NOWAIT cases as well, and
> if it's actually set, it'll be handled by the !PageUpdtodate case.
>
> > Of course, there aren't any tests for RWF_NOWAIT in xfstests. Are there
> > any in LTP?
>
> Not in the released version AFAIK. I've asked the LTP maintainer (in our
> internal bugzilla) to take care of this thread a few days ago, but not
> sure what came out of it. Adding him (Cyril) to CC.
So far not much, I've looked over our mincore() tests and noted down how
to improve them here:
https://github.com/linux-test-project/ltp/issues/461
We do plan to test the final mincore() fix:
https://github.com/linux-test-project/ltp/issues/460
And we do have RWF_NOWAIT tests on our TODO for some time as well:
https://github.com/linux-test-project/ltp/issues/286
I guess I can raise priority for that one so that we have basic
functional tests in a week or so. Also if anyone has some RWF_NOWAIT
tests already it would be nice if these could be shared with us.
[A bit off topic rant]
I've been telling kernel developers for years that if they have a test
code they used when developing a kernel feature that they should share
it with us (LTP community) and we will turn these into automated tests
and maintain them for free. LTP is also used in many QA departements
around the word so such tests will end up executed in different
environments also for free. Sadly this does not happen much and there
are only few exceptions so far. But maybe I wasn't shouting loudly
enough.
--
Cyril Hrubis
chrubis@suse.cz
^ permalink raw reply
* Re: [PATCH] mm/mincore: allow for making sys_mincore() privileged
From: Jiri Kosina @ 2019-01-17 8:18 UTC (permalink / raw)
To: Dave Chinner
Cc: Linus Torvalds, Matthew Wilcox, Jann Horn, Andrew Morton, Greg KH,
Peter Zijlstra, Michal Hocko, Linux-MM, kernel list, Linux API
In-Reply-To: <20190117022244.GV4205@dastard>
On Thu, 17 Jan 2019, Dave Chinner wrote:
> > > commit e837eac23662afae603aaaef7c94bc839c1b8f67
> > > Author: Steve Lord <lord@sgi.com>
> > > Date: Mon Mar 5 16:47:52 2001 +0000
> > >
> > > Add bounds checking for direct I/O, do the cache invalidation for
> > > data coherency on direct I/O.
> >
> > Out of curiosity, which repository is this from please? Even google
> > doesn't seem to know about this SHA.
>
> because oss.sgi.com is no longer with us, it's fallen out of all the
> search engines. It was from the "archive/xfs-import.git" tree on
> oss.sgi.com:
>
> https://web.archive.org/web/20120326044237/http://oss.sgi.com:80/cgi-bin/gitweb.cgi
>
> but archive.org doesn't have a copy of the git tree. It contained
> the XFS history right back to the first Irix commit in 1993. Some of
> us still have copies of it sitting around....
For cases like this, would it be worth pushing it to git.kernel.org as an
frozen historical reference archive?
Thanks,
--
Jiri Kosina
SUSE Labs
^ permalink raw reply
* Re: [PATCH] mm/mincore: allow for making sys_mincore() privileged
From: Linus Torvalds @ 2019-01-17 4:51 UTC (permalink / raw)
To: Matthew Wilcox
Cc: Jiri Kosina, Dominique Martinet, Andy Lutomirski, Josh Snyder,
Dave Chinner, Jann Horn, Andrew Morton, Greg KH, Peter Zijlstra,
Michal Hocko, Linux-MM, kernel list, Linux API
In-Reply-To: <20190116213708.GN6310@bombadil.infradead.org>
On Thu, Jan 17, 2019 at 9:37 AM Matthew Wilcox <willy@infradead.org> wrote:
>
> Your patch 3/3 just removes the test. Am I right in thinking that it
> doesn't need to be *moved* because the existing test after !PageUptodate
> catches it?
That's the _hope_.
That's the simplest patch I can come up with as a potential solution.
But it's possible that there's some nasty performance regression
because somebody really relies on not even triggering read-ahead, and
we might need to do some totally different thing.
So it may be that somebody has a case that really wants something
else, and we'd need to move the RWF_NOWAIT test elsewhere and do
something slightly more complicated. As with the mincore() change,
maybe reality doesn't like the simplest fix...
> Of course, there aren't any tests for RWF_NOWAIT in xfstests. Are there
> any in LTP?
RWF_NOWAIT is actually _fairly_ new. It was introduced "only" about
18 months ago and made it into 4.13.
Which makes me hopeful there aren't a lot of people who care deeply.
And starting readahead *may* actually be what a RWF_NOWAIT read user
generally wants, so for all we know it might even improve performance
and/or allow new uses. With the "start readahead but don't wait for
it" semantics, you can have a model where you try to handle all the
requests that can be handled out of cache first (using RWF_NOWAIT) and
then when you've run out of cached cases you clear the RWF_NOWAIT
flag, but now the IO has been started early (and could overlap with
the cached request handling), so then when you actually do a blocking
version, you get much better performance.
So there is an argument that removing that one RWF_NOWAIT case might
actually be a good thing in general, outside of the "don't allow
probing the cache without changing the state of it" issue.
But that's handwavy and optimistic. Reality is often not as accomodating ;)
Linus
^ permalink raw reply
* Re: [PATCH] mm/mincore: allow for making sys_mincore() privileged
From: Dave Chinner @ 2019-01-17 2:22 UTC (permalink / raw)
To: Jiri Kosina
Cc: Linus Torvalds, Matthew Wilcox, Jann Horn, Andrew Morton, Greg KH,
Peter Zijlstra, Michal Hocko, Linux-MM, kernel list, Linux API
In-Reply-To: <nycvar.YFH.7.76.1901110836110.6626@cbobk.fhfr.pm>
On Fri, Jan 11, 2019 at 08:36:55AM +0100, Jiri Kosina wrote:
> On Thu, 10 Jan 2019, Dave Chinner wrote:
>
> > Sounds nice from a theoretical POV, but reality has taught us very
> > different lessons.
> >
> > FWIW, a quick check of XFS's history so you understand how long this
> > behaviour has been around. It was introduced in the linux port in 2001
> > as direct IO support was being added:
> >
> > commit e837eac23662afae603aaaef7c94bc839c1b8f67
> > Author: Steve Lord <lord@sgi.com>
> > Date: Mon Mar 5 16:47:52 2001 +0000
> >
> > Add bounds checking for direct I/O, do the cache invalidation for
> > data coherency on direct I/O.
>
> Out of curiosity, which repository is this from please? Even google
> doesn't seem to know about this SHA.
because oss.sgi.com is no longer with us, it's fallen out of all the
search engines. It was from the "archive/xfs-import.git" tree on
oss.sgi.com:
https://web.archive.org/web/20120326044237/http://oss.sgi.com:80/cgi-bin/gitweb.cgi
but archive.org doesn't have a copy of the git tree. It contained
the XFS history right back to the first Irix commit in 1993. Some of
us still have copies of it sitting around....
Cheers,
Dave.
--
Dave Chinner
david@fromorbit.com
^ permalink raw reply
* Re: [PATCH] mm/mincore: allow for making sys_mincore() privileged
From: Dominique Martinet @ 2019-01-17 1:49 UTC (permalink / raw)
To: Jiri Kosina
Cc: Linus Torvalds, Andy Lutomirski, Josh Snyder, Dave Chinner,
Matthew Wilcox, Jann Horn, Andrew Morton, Greg KH, Peter Zijlstra,
Michal Hocko, Linux-MM, kernel list, Linux API
In-Reply-To: <nycvar.YFH.7.76.1901162120000.6626@cbobk.fhfr.pm>
Jiri Kosina wrote on Wed, Jan 16, 2019:
> So if noone sees any principal problem there, I'll happily submit it with
> proper attribution etc.
I'm not convinced just the write permission check is enough for
mincore(), as Josh also seems to share the concern I raised (e.g. map a
git directory "hot" pages)
We probably need to add an inode_owner_or_capable() or similar, the open
question is do we still need the write access check after that - I don't
really know how expensive these calls are.
Thanks,
--
Dominique
^ permalink raw reply
* Re: [PATCH] mm/mincore: allow for making sys_mincore() privileged
From: Dave Chinner @ 2019-01-17 1:26 UTC (permalink / raw)
To: Linus Torvalds
Cc: Dominique Martinet, Jiri Kosina, Matthew Wilcox, Jann Horn,
Andrew Morton, Greg KH, Peter Zijlstra, Michal Hocko, Linux-MM,
kernel list, Linux API
In-Reply-To: <CAHk-=wjc2inOae8+9-DK4jFK78-7ZpNR=TEyZg0Dj57SYwP-ng@mail.gmail.com>
On Wed, Jan 16, 2019 at 04:54:49PM +1200, Linus Torvalds wrote:
> On Wed, Jan 16, 2019 at 11:45 AM Dave Chinner <david@fromorbit.com> wrote:
> >
> > I'm assuming that you can invalidate the page cache reliably by a
> > means that does not repeated require probing to detect invalidation
> > has occurred. I've mentioned one method in this discussion
> > already...
>
> Yes. And it was made clear to you that it was a bug in xfs dio and
> what the right thing to do was.
>
> And you ignored that, and claimed it was a feature.
Linus, either you aren't listening or you're being intentionally
provocative.
So, for the *third* time this thread: we can probably remove this
code but first we need to be sure it doesn't cause unexpected
regressions before we commit such a change. We are not cowboys who
test userspace behavioural changes on users without review or
discussion.
Indeed, I wrote a patch to remove the invalidation /several days
ago/ and put it into my test trees, and it's been there since. Just
because you don't see immediate changes doesn't mean it isn't
happening.
> Either you care or you don't. If you don't care (and so far everything
> you said seems to imply you don't),
Linus, this is just a personal attack and IMO a violation of the
CoC. It's straight out wrong, insulting, totally unprofessional and
completely uncalled for.
This is most definitely not a useful technical response to the
issues I raised. i.e you cut out all the context of my response
about whether "no probing necessary" page cache invalidation attacks
are something we need to care about in the future. We don't need you
to shout about existing "no probing necessary" page cache
invalidation attacks that are already being addressed, we need to
determine if it's going to be a recurring problem in future because
that directly affects the mitigation strategies we can implement.
-Dave.
--
Dave Chinner
david@fromorbit.com
^ permalink raw reply
* Re: [PATCH] mm/mincore: allow for making sys_mincore() privileged
From: Jiri Kosina @ 2019-01-16 21:41 UTC (permalink / raw)
To: Matthew Wilcox
Cc: Linus Torvalds, Dominique Martinet, Andy Lutomirski, Josh Snyder,
Dave Chinner, Jann Horn, Andrew Morton, Greg KH, Peter Zijlstra,
Michal Hocko, Linux-MM, kernel list, Cyril Hrubis, Linux API
In-Reply-To: <20190116213708.GN6310@bombadil.infradead.org>
On Wed, 16 Jan 2019, Matthew Wilcox wrote:
> > On Thu, 17 Jan 2019, Linus Torvalds wrote:
> > > As I suggested earlier in the thread, the fix for RWF_NOWAIT might be
> > > to just move the test down to after readahead.
>
> Your patch 3/3 just removes the test. Am I right in thinking that it
> doesn't need to be *moved* because the existing test after !PageUptodate
> catches it?
Exactly. It just initiates read-ahead for IOCB_NOWAIT cases as well, and
if it's actually set, it'll be handled by the !PageUpdtodate case.
> Of course, there aren't any tests for RWF_NOWAIT in xfstests. Are there
> any in LTP?
Not in the released version AFAIK. I've asked the LTP maintainer (in our
internal bugzilla) to take care of this thread a few days ago, but not
sure what came out of it. Adding him (Cyril) to CC.
> Some typos in the commit messages:
>
> > Another aproach (checking file access permissions in order to decide
> "approach"
>
> > Subject: [PATCH 2/3] mm/mincore: make mincore() more conservative
> >
> > The semantics of what mincore() considers to be resident is not completely
> > clearar, but Linux has always (since 2.3.52, which is when mincore() was
> "clear"
>
> > initially done) treated it as "page is available in page cache".
> >
> > That's potentially a problem, as that [in]directly exposes meta-information
> > about pagecache / memory mapping state even about memory not strictly belonging
> > to the process executing the syscall, opening possibilities for sidechannel
> > attacks.
> >
> > Change the semantics of mincore() so that it only reveals pagecache information
> > for non-anonymous mappings that belog to files that the calling process could
> "belong"
Thanks.
--
Jiri Kosina
SUSE Labs
^ permalink raw reply
* Re: [PATCH] mm/mincore: allow for making sys_mincore() privileged
From: Matthew Wilcox @ 2019-01-16 21:37 UTC (permalink / raw)
To: Jiri Kosina
Cc: Linus Torvalds, Dominique Martinet, Andy Lutomirski, Josh Snyder,
Dave Chinner, Jann Horn, Andrew Morton, Greg KH, Peter Zijlstra,
Michal Hocko, Linux-MM, kernel list, Linux API
In-Reply-To: <nycvar.YFH.7.76.1901162120000.6626@cbobk.fhfr.pm>
On Wed, Jan 16, 2019 at 09:23:04PM +0100, Jiri Kosina wrote:
> On Thu, 17 Jan 2019, Linus Torvalds wrote:
> > As I suggested earlier in the thread, the fix for RWF_NOWAIT might be
> > to just move the test down to after readahead.
Your patch 3/3 just removes the test. Am I right in thinking that it
doesn't need to be *moved* because the existing test after !PageUptodate
catches it?
Of course, there aren't any tests for RWF_NOWAIT in xfstests. Are there
any in LTP?
Some typos in the commit messages:
> Another aproach (checking file access permissions in order to decide
"approach"
> Subject: [PATCH 2/3] mm/mincore: make mincore() more conservative
>
> The semantics of what mincore() considers to be resident is not completely
> clearar, but Linux has always (since 2.3.52, which is when mincore() was
"clear"
> initially done) treated it as "page is available in page cache".
>
> That's potentially a problem, as that [in]directly exposes meta-information
> about pagecache / memory mapping state even about memory not strictly belonging
> to the process executing the syscall, opening possibilities for sidechannel
> attacks.
>
> Change the semantics of mincore() so that it only reveals pagecache information
> for non-anonymous mappings that belog to files that the calling process could
"belong"
^ permalink raw reply
* Re: [PATCH] mm/mincore: allow for making sys_mincore() privileged
From: Jiri Kosina @ 2019-01-16 20:23 UTC (permalink / raw)
To: Linus Torvalds
Cc: Dominique Martinet, Andy Lutomirski, Josh Snyder, Dave Chinner,
Matthew Wilcox, Jann Horn, Andrew Morton, Greg KH, Peter Zijlstra,
Michal Hocko, Linux-MM, kernel list, Linux API
In-Reply-To: <CAHk-=wgsnWvSsMfoEYzOq6fpahkHWxF3aSJBbVqywLa34OXnLg@mail.gmail.com>
[-- Attachment #1: Type: text/plain, Size: 801 bytes --]
On Thu, 17 Jan 2019, Linus Torvalds wrote:
> > So that seems to deal with mincore() in a reasonable way indeed.
> >
> > It doesn't unfortunately really solve the preadv2(RWF_NOWAIT), nor does it
> > provide any good answer what to do about it, does it?
>
> As I suggested earlier in the thread, the fix for RWF_NOWAIT might be
> to just move the test down to after readahead.
So I've done some basic smoke testing (~2 hours of LTP+xfstests) on the
kernel with the three topmost patches from
https://git.kernel.org/pub/scm/linux/kernel/git/jikos/jikos.git/log/?h=pagecache-sidechannel
applied (also attaching to this mail), and no obvious breakage popped up.
So if noone sees any principal problem there, I'll happily submit it with
proper attribution etc.
Thanks,
--
Jiri Kosina
SUSE Labs
[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: Type: text/x-patch; name=0001-Revert-Change-mincore-to-count-mapped-pages-rather-t.patch, Size: 4248 bytes --]
From cbf9381eed6766cff5b05f9d948c1d225cb3d78b Mon Sep 17 00:00:00 2001
From: Jiri Kosina <jkosina@suse.cz>
Date: Wed, 16 Jan 2019 20:51:31 +0100
Subject: [PATCH 1/3] Revert "Change mincore() to count "mapped" pages rather
than "cached" pages"
This reverts commit 574823bfab82d9d8fa47f422778043fbb4b4f50e.
Another aproach (checking file access permissions in order to decide
what mincore() should return in order not to leak data) will be implemented
instead.
Signed-off-by: Jiri Kosina <jkosina@suse.cz>
---
mm/mincore.c | 94 +++++++++++++++++++++++++++++++++++++++++++++++++++---------
1 file changed, 81 insertions(+), 13 deletions(-)
diff --git a/mm/mincore.c b/mm/mincore.c
index f0f91461a9f4..218099b5ed31 100644
--- a/mm/mincore.c
+++ b/mm/mincore.c
@@ -42,14 +42,72 @@ static int mincore_hugetlb(pte_t *pte, unsigned long hmask, unsigned long addr,
return 0;
}
-static int mincore_unmapped_range(unsigned long addr, unsigned long end,
- struct mm_walk *walk)
+/*
+ * Later we can get more picky about what "in core" means precisely.
+ * For now, simply check to see if the page is in the page cache,
+ * and is up to date; i.e. that no page-in operation would be required
+ * at this time if an application were to map and access this page.
+ */
+static unsigned char mincore_page(struct address_space *mapping, pgoff_t pgoff)
+{
+ unsigned char present = 0;
+ struct page *page;
+
+ /*
+ * When tmpfs swaps out a page from a file, any process mapping that
+ * file will not get a swp_entry_t in its pte, but rather it is like
+ * any other file mapping (ie. marked !present and faulted in with
+ * tmpfs's .fault). So swapped out tmpfs mappings are tested here.
+ */
+#ifdef CONFIG_SWAP
+ if (shmem_mapping(mapping)) {
+ page = find_get_entry(mapping, pgoff);
+ /*
+ * shmem/tmpfs may return swap: account for swapcache
+ * page too.
+ */
+ if (xa_is_value(page)) {
+ swp_entry_t swp = radix_to_swp_entry(page);
+ page = find_get_page(swap_address_space(swp),
+ swp_offset(swp));
+ }
+ } else
+ page = find_get_page(mapping, pgoff);
+#else
+ page = find_get_page(mapping, pgoff);
+#endif
+ if (page) {
+ present = PageUptodate(page);
+ put_page(page);
+ }
+
+ return present;
+}
+
+static int __mincore_unmapped_range(unsigned long addr, unsigned long end,
+ struct vm_area_struct *vma, unsigned char *vec)
{
- unsigned char *vec = walk->private;
unsigned long nr = (end - addr) >> PAGE_SHIFT;
+ int i;
- memset(vec, 0, nr);
- walk->private += nr;
+ if (vma->vm_file) {
+ pgoff_t pgoff;
+
+ pgoff = linear_page_index(vma, addr);
+ for (i = 0; i < nr; i++, pgoff++)
+ vec[i] = mincore_page(vma->vm_file->f_mapping, pgoff);
+ } else {
+ for (i = 0; i < nr; i++)
+ vec[i] = 0;
+ }
+ return nr;
+}
+
+static int mincore_unmapped_range(unsigned long addr, unsigned long end,
+ struct mm_walk *walk)
+{
+ walk->private += __mincore_unmapped_range(addr, end,
+ walk->vma, walk->private);
return 0;
}
@@ -69,9 +127,8 @@ static int mincore_pte_range(pmd_t *pmd, unsigned long addr, unsigned long end,
goto out;
}
- /* We'll consider a THP page under construction to be there */
if (pmd_trans_unstable(pmd)) {
- memset(vec, 1, nr);
+ __mincore_unmapped_range(addr, end, vma, vec);
goto out;
}
@@ -80,17 +137,28 @@ static int mincore_pte_range(pmd_t *pmd, unsigned long addr, unsigned long end,
pte_t pte = *ptep;
if (pte_none(pte))
- *vec = 0;
+ __mincore_unmapped_range(addr, addr + PAGE_SIZE,
+ vma, vec);
else if (pte_present(pte))
*vec = 1;
else { /* pte is a swap entry */
swp_entry_t entry = pte_to_swp_entry(pte);
- /*
- * migration or hwpoison entries are always
- * uptodate
- */
- *vec = !!non_swap_entry(entry);
+ if (non_swap_entry(entry)) {
+ /*
+ * migration or hwpoison entries are always
+ * uptodate
+ */
+ *vec = 1;
+ } else {
+#ifdef CONFIG_SWAP
+ *vec = mincore_page(swap_address_space(entry),
+ swp_offset(entry));
+#else
+ WARN_ON(1);
+ *vec = 1;
+#endif
+ }
}
vec++;
}
--
2.12.3
[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #3: Type: text/x-patch; name=0002-mm-mincore-make-mincore-more-conservative.patch, Size: 2315 bytes --]
From ca02a026f40dfaebc29c29edd9c992a0ff10075e Mon Sep 17 00:00:00 2001
From: Jiri Kosina <jkosina@suse.cz>
Date: Wed, 16 Jan 2019 20:53:17 +0100
Subject: [PATCH 2/3] mm/mincore: make mincore() more conservative
The semantics of what mincore() considers to be resident is not completely
clearar, but Linux has always (since 2.3.52, which is when mincore() was
initially done) treated it as "page is available in page cache".
That's potentially a problem, as that [in]directly exposes meta-information
about pagecache / memory mapping state even about memory not strictly belonging
to the process executing the syscall, opening possibilities for sidechannel
attacks.
Change the semantics of mincore() so that it only reveals pagecache information
for non-anonymous mappings that belog to files that the calling process could
(if it tried to) successfully open for writing.
Originally-by: Linus Torvalds <torvalds@linux-foundation.org>
Originally-by: Dominique Martinet <asmadeus@codewreck.org>
Signed-off-by: Jiri Kosina <jkosina@suse.cz>
---
mm/mincore.c | 14 +++++++++++++-
1 file changed, 13 insertions(+), 1 deletion(-)
diff --git a/mm/mincore.c b/mm/mincore.c
index 218099b5ed31..11ed7064f4eb 100644
--- a/mm/mincore.c
+++ b/mm/mincore.c
@@ -169,6 +169,13 @@ static int mincore_pte_range(pmd_t *pmd, unsigned long addr, unsigned long end,
return 0;
}
+static inline bool can_do_mincore(struct vm_area_struct *vma)
+{
+ return vma_is_anonymous(vma)
+ || (vma->vm_file && (vma->vm_file->f_mode & FMODE_WRITE))
+ || inode_permission(file_inode(vma->vm_file), MAY_WRITE) == 0;
+}
+
/*
* Do a chunk of "sys_mincore()". We've already checked
* all the arguments, we hold the mmap semaphore: we should
@@ -189,8 +196,13 @@ static long do_mincore(unsigned long addr, unsigned long pages, unsigned char *v
vma = find_vma(current->mm, addr);
if (!vma || addr < vma->vm_start)
return -ENOMEM;
- mincore_walk.mm = vma->vm_mm;
end = min(vma->vm_end, addr + (pages << PAGE_SHIFT));
+ if (!can_do_mincore(vma)) {
+ unsigned long pages = (end - addr) >> PAGE_SHIFT;
+ memset(vec, 1, pages);
+ return pages;
+ }
+ mincore_walk.mm = vma->vm_mm;
err = walk_page_range(addr, end, &mincore_walk);
if (err < 0)
return err;
--
2.12.3
[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #4: Type: text/x-patch; name=0003-mm-filemap-initiate-readahead-even-if-IOCB_NOWAIT-is.patch, Size: 1368 bytes --]
From e7765f317afb193adb4ba00d81251686191cbf4b Mon Sep 17 00:00:00 2001
From: Jiri Kosina <jkosina@suse.cz>
Date: Wed, 16 Jan 2019 21:06:58 +0100
Subject: [PATCH 3/3] mm/filemap: initiate readahead even if IOCB_NOWAIT is set
for the I/O
preadv2(RWF_NOWAIT) can be used to open a side-channel to pagecache contents, as
it reveals metadata about residency of pages in pagecache.
If preadv2(RWF_NOWAIT) returns immediately, it provides a clear "page not
resident" information, and vice versa.
Close that sidechannel by always initiating readahead on the cache if we
encounter a cache miss for preadv2(RWF_NOWAIT); with that in place, probing
the pagecache residency itself will actually populate the cache, making the
sidechannel useless.
Originally-by: Linus Torvalds <torvalds@linux-foundation.org>
Signed-off-by: Jiri Kosina <jkosina@suse.cz>
---
mm/filemap.c | 2 --
1 file changed, 2 deletions(-)
diff --git a/mm/filemap.c b/mm/filemap.c
index 9f5e323e883e..7bcdd36e629d 100644
--- a/mm/filemap.c
+++ b/mm/filemap.c
@@ -2075,8 +2075,6 @@ static ssize_t generic_file_buffered_read(struct kiocb *iocb,
page = find_get_page(mapping, index);
if (!page) {
- if (iocb->ki_flags & IOCB_NOWAIT)
- goto would_block;
page_cache_sync_readahead(mapping,
ra, filp,
index, last_index - index);
--
2.12.3
^ permalink raw reply related
* Re: [PATCH] mm/mincore: allow for making sys_mincore() privileged
From: Linus Torvalds @ 2019-01-16 17:48 UTC (permalink / raw)
To: Jiri Kosina
Cc: Dominique Martinet, Andy Lutomirski, Josh Snyder, Dave Chinner,
Matthew Wilcox, Jann Horn, Andrew Morton, Greg KH, Peter Zijlstra,
Michal Hocko, Linux-MM, kernel list, Linux API
In-Reply-To: <nycvar.YFH.7.76.1901161710470.6626@cbobk.fhfr.pm>
On Thu, Jan 17, 2019 at 4:12 AM Jiri Kosina <jikos@kernel.org> wrote:
>
> So that seems to deal with mincore() in a reasonable way indeed.
>
> It doesn't unfortunately really solve the preadv2(RWF_NOWAIT), nor does it
> provide any good answer what to do about it, does it?
As I suggested earlier in the thread, the fix for RWF_NOWAIT might be
to just move the test down to after readahead.
We could/should be smarter than this,but it *might* be as simple as just
diff --git a/mm/filemap.c b/mm/filemap.c
index 9f5e323e883e..7bcdd36e629d 100644
--- a/mm/filemap.c
+++ b/mm/filemap.c
@@ -2075,8 +2075,6 @@ static ssize_t generic_file_buffered_read(
page = find_get_page(mapping, index);
if (!page) {
- if (iocb->ki_flags & IOCB_NOWAIT)
- goto would_block;
page_cache_sync_readahead(mapping,
ra, filp,
index, last_index - index);
which starts readahead even if IOCB_NOWAIT is set and will then test
IOCB_NOWAIT _later_ and not actually wait for it.
Linus
^ permalink raw reply related
* Re: [PATCH] mm/mincore: allow for making sys_mincore() privileged
From: Jiri Kosina @ 2019-01-16 16:12 UTC (permalink / raw)
To: Linus Torvalds
Cc: Dominique Martinet, Andy Lutomirski, Josh Snyder, Dave Chinner,
Matthew Wilcox, Jann Horn, Andrew Morton, Greg KH, Peter Zijlstra,
Michal Hocko, Linux-MM, kernel list, Linux API
In-Reply-To: <CAHk-=wjVjecbGRcxZUSwoSgAq9ZbMxbA=MOiqDrPgx7_P3xGhg@mail.gmail.com>
On Wed, 16 Jan 2019, Linus Torvalds wrote:
> > "Being owner or has cap" (whichever cap) is probably OK. On the other
> > hand, writeability check makes more sense in general - could we
> > somehow check if the user has write access to the file instead of
> > checking if it currently is opened read-write?
>
> That's likely the best option. We could say "is it open for write, or
> _could_ we open it for writing?"
>
> It's a slightly annoying special case, and I'd have preferred to avoid
> it, but it doesn't sound *compilcated*.
>
> I'm on the road, but I did send out this:
>
> https://lore.kernel.org/lkml/CAHk-=wif_9nvNHJiyxHzJ80_WUb0P7CXNBvXkjZz-r1u0ozp7g@mail.gmail.com/
>
> originally. The "let's try to only do the mmap residency" was the
> optimistic "maybe we can just get rid of this complexity entirely"
> version..
>
> Anybody willing to test the above patch instead? And replace the
>
> || capable(CAP_SYS_ADMIN)
>
> check with something like
>
> || inode_permission(inode, MAY_WRITE) == 0
>
> instead?
>
> (This is obviously after you've reverted the "only check mmap
> residency" patch..)
So that seems to deal with mincore() in a reasonable way indeed.
It doesn't unfortunately really solve the preadv2(RWF_NOWAIT), nor does it
provide any good answer what to do about it, does it?
Thanks,
--
Jiri Kosina
SUSE Labs
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox