| Message ID | 20240216051640.197378-2-vinicius.gomes@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | overlayfs: Optimize override/revert creds | expand |
On Thu, Feb 15, 2024 at 09:16:36PM -0800, Vinicius Costa Gomes wrote: > Fix the following warning when defining a cleanup guard for a "const" > pointer type: > > ./include/linux/cleanup.h:211:18: warning: return discards ‘const’ qualifier from pointer target type [-Wdiscarded-qualifiers] > 211 | return _T->lock; \ > | ~~^~~~~~ > ./include/linux/cleanup.h:233:1: note: in expansion of macro ‘__DEFINE_UNLOCK_GUARD’ > 233 | __DEFINE_UNLOCK_GUARD(_name, _type, _unlock, __VA_ARGS__) \ > | ^~~~~~~~~~~~~~~~~~~~~ > ./include/linux/cred.h:193:1: note: in expansion of macro ‘DEFINE_LOCK_GUARD_1’ > 193 | DEFINE_LOCK_GUARD_1(cred, const struct cred, _T->lock = override_creds_light(_T->lock), > | ^~~~~~~~~~~~~~~~~~~ > > Signed-off-by: Vinicius Costa Gomes <vinicius.gomes@intel.com> > --- > include/linux/cleanup.h | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/include/linux/cleanup.h b/include/linux/cleanup.h > index c2d09bc4f976..085482ef46c8 100644 > --- a/include/linux/cleanup.h > +++ b/include/linux/cleanup.h > @@ -208,7 +208,7 @@ static inline void class_##_name##_destructor(class_##_name##_t *_T) \ > \ > static inline void *class_##_name##_lock_ptr(class_##_name##_t *_T) \ > { \ > - return _T->lock; \ > + return (void *)_T->lock; \ > } I think both of these patches are a bit ugly as we burden the generic cleanup code with casting to void which could cause actual issues. Casting from const to non-const is rather specific to the cred code so I would rather like to put the burden on the cred code instead of the generic code if possible.
On Mon, Mar 18, 2024 at 04:13:12PM +0100, Christian Brauner wrote: > On Thu, Feb 15, 2024 at 09:16:36PM -0800, Vinicius Costa Gomes wrote: > > Fix the following warning when defining a cleanup guard for a "const" > > pointer type: > > > > ./include/linux/cleanup.h:211:18: warning: return discards ‘const’ qualifier from pointer target type [-Wdiscarded-qualifiers] > > 211 | return _T->lock; \ > > | ~~^~~~~~ > > ./include/linux/cleanup.h:233:1: note: in expansion of macro ‘__DEFINE_UNLOCK_GUARD’ > > 233 | __DEFINE_UNLOCK_GUARD(_name, _type, _unlock, __VA_ARGS__) \ > > | ^~~~~~~~~~~~~~~~~~~~~ > > ./include/linux/cred.h:193:1: note: in expansion of macro ‘DEFINE_LOCK_GUARD_1’ > > 193 | DEFINE_LOCK_GUARD_1(cred, const struct cred, _T->lock = override_creds_light(_T->lock), > > | ^~~~~~~~~~~~~~~~~~~ > > > > Signed-off-by: Vinicius Costa Gomes <vinicius.gomes@intel.com> > > --- > > include/linux/cleanup.h | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > diff --git a/include/linux/cleanup.h b/include/linux/cleanup.h > > index c2d09bc4f976..085482ef46c8 100644 > > --- a/include/linux/cleanup.h > > +++ b/include/linux/cleanup.h > > @@ -208,7 +208,7 @@ static inline void class_##_name##_destructor(class_##_name##_t *_T) \ > > \ > > static inline void *class_##_name##_lock_ptr(class_##_name##_t *_T) \ > > { \ > > - return _T->lock; \ > > + return (void *)_T->lock; \ > > } > > I think both of these patches are a bit ugly as we burden the generic > cleanup code with casting to void which could cause actual issues. > > Casting from const to non-const is rather specific to the cred code so I > would rather like to put the burden on the cred code instead of the > generic code if possible. So something like this? (Amir?) diff --git a/fs/backing-file.c b/fs/backing-file.c index ed00ce93cad2..9610d5166736 100644 --- a/fs/backing-file.c +++ b/fs/backing-file.c @@ -152,7 +152,7 @@ ssize_t backing_file_read_iter(struct file *file, struct iov_iter *iter, !(file->f_mode & FMODE_CAN_ODIRECT)) return -EINVAL; - guard(cred)(ctx->cred); + cred_guard(ctx->cred); if (is_sync_kiocb(iocb)) { rwf_t rwf = iocb_to_rw_flags(flags); @@ -206,7 +206,7 @@ ssize_t backing_file_write_iter(struct file *file, struct iov_iter *iter, */ flags &= ~IOCB_DIO_CALLER_COMP; - guard(cred)(ctx->cred); + cred_guard(ctx->cred); if (is_sync_kiocb(iocb)) { rwf_t rwf = iocb_to_rw_flags(flags); @@ -250,7 +250,7 @@ ssize_t backing_file_splice_read(struct file *in, loff_t *ppos, if (WARN_ON_ONCE(!(in->f_mode & FMODE_BACKING))) return -EIO; - guard(cred)(ctx->cred); + cred_guard(ctx->cred); ret = vfs_splice_read(in, ppos, pipe, len, flags); if (ctx->accessed) @@ -274,7 +274,7 @@ ssize_t backing_file_splice_write(struct pipe_inode_info *pipe, if (ret) return ret; - guard(cred)(ctx->cred); + cred_guard(ctx->cred); file_start_write(out); ret = iter_file_splice_write(pipe, out, ppos, len, flags); file_end_write(out); @@ -300,7 +300,7 @@ int backing_file_mmap(struct file *file, struct vm_area_struct *vma, vma_set_file(vma, file); - guard(cred)(ctx->cred); + cred_guard(ctx->cred); ret = call_mmap(vma->vm_file, vma); if (ctx->accessed) diff --git a/fs/overlayfs/copy_up.c b/fs/overlayfs/copy_up.c index 192997837a56..156d0262ddad 100644 --- a/fs/overlayfs/copy_up.c +++ b/fs/overlayfs/copy_up.c @@ -1199,7 +1199,7 @@ static int ovl_copy_up_flags(struct dentry *dentry, int flags) if (err) return err; - guard(cred)(ovl_creds(dentry->d_sb)); + cred_guard(ovl_creds(dentry->d_sb)); while (!err) { struct dentry *next; struct dentry *parent = NULL; diff --git a/fs/overlayfs/dir.c b/fs/overlayfs/dir.c index 3ea5ba7b980d..d9497e34b4c8 100644 --- a/fs/overlayfs/dir.c +++ b/fs/overlayfs/dir.c @@ -686,7 +686,7 @@ static int ovl_set_link_redirect(struct dentry *dentry) { int err; - guard(cred)(ovl_creds(dentry->d_sb)); + cred_guard(ovl_creds(dentry->d_sb)); err = ovl_set_redirect(dentry, false); return err; @@ -891,7 +891,7 @@ static int ovl_do_remove(struct dentry *dentry, bool is_dir) if (err) goto out; - guard(cred)(ovl_creds(dentry->d_sb)); + cred_guard(ovl_creds(dentry->d_sb)); if (!lower_positive) err = ovl_remove_upper(dentry, is_dir, &list); else diff --git a/fs/overlayfs/file.c b/fs/overlayfs/file.c index 44744196bf21..f97d4b106d52 100644 --- a/fs/overlayfs/file.c +++ b/fs/overlayfs/file.c @@ -41,7 +41,7 @@ static struct file *ovl_open_realfile(const struct file *file, if (flags & O_APPEND) acc_mode |= MAY_APPEND; - guard(cred)(ovl_creds(inode->i_sb)); + cred_guard(ovl_creds(inode->i_sb)); real_idmap = mnt_idmap(realpath->mnt); err = inode_permission(real_idmap, realinode, MAY_OPEN | acc_mode); if (err) { @@ -211,7 +211,7 @@ static loff_t ovl_llseek(struct file *file, loff_t offset, int whence) ovl_inode_lock(inode); real.file->f_pos = file->f_pos; - guard(cred)(ovl_creds(inode->i_sb)); + cred_guard(ovl_creds(inode->i_sb)); ret = vfs_llseek(real.file, offset, whence); file->f_pos = real.file->f_pos; @@ -396,7 +396,7 @@ static int ovl_fsync(struct file *file, loff_t start, loff_t end, int datasync) /* Don't sync lower file for fear of receiving EROFS error */ if (file_inode(real.file) == ovl_inode_upper(file_inode(file))) { - guard(cred)(ovl_creds(file_inode(file)->i_sb)); + cred_guard(ovl_creds(file_inode(file)->i_sb)); ret = vfs_fsync_range(real.file, start, end, datasync); } @@ -434,7 +434,7 @@ static long ovl_fallocate(struct file *file, int mode, loff_t offset, loff_t len if (ret) goto out_unlock; - guard(cred)(ovl_creds(file_inode(file)->i_sb)); + cred_guard(ovl_creds(file_inode(file)->i_sb)); ret = vfs_fallocate(real.file, mode, offset, len); /* Update size */ @@ -457,7 +457,7 @@ static int ovl_fadvise(struct file *file, loff_t offset, loff_t len, int advice) if (ret) return ret; - guard(cred)(ovl_creds(file_inode(file)->i_sb)); + cred_guard(ovl_creds(file_inode(file)->i_sb)); ret = vfs_fadvise(real.file, offset, len, advice); fdput(real); @@ -498,7 +498,7 @@ static loff_t ovl_copyfile(struct file *file_in, loff_t pos_in, goto out_unlock; } - guard(cred)(ovl_creds(file_inode(file_out)->i_sb)); + cred_guard(ovl_creds(file_inode(file_out)->i_sb)); switch (op) { case OVL_COPY: ret = vfs_copy_file_range(real_in.file, pos_in, @@ -574,7 +574,7 @@ static int ovl_flush(struct file *file, fl_owner_t id) return err; if (real.file->f_op->flush) { - guard(cred)(ovl_creds(file_inode(file)->i_sb)); + cred_guard(ovl_creds(file_inode(file)->i_sb)); err = real.file->f_op->flush(real.file, id); } fdput(real); diff --git a/fs/overlayfs/inode.c b/fs/overlayfs/inode.c index 8e399b10ebba..2f3024d2a119 100644 --- a/fs/overlayfs/inode.c +++ b/fs/overlayfs/inode.c @@ -78,7 +78,7 @@ int ovl_setattr(struct mnt_idmap *idmap, struct dentry *dentry, goto out_put_write; inode_lock(upperdentry->d_inode); - guard(cred)(ovl_creds(dentry->d_sb)); + cred_guard(ovl_creds(dentry->d_sb)); err = ovl_do_notify_change(ofs, upperdentry, attr); @@ -169,7 +169,7 @@ int ovl_getattr(struct mnt_idmap *idmap, const struct path *path, metacopy_blocks = ovl_is_metacopy_dentry(dentry); type = ovl_path_real(dentry, &realpath); - guard(cred)(ovl_creds(dentry->d_sb)); + cred_guard(ovl_creds(dentry->d_sb)); err = ovl_do_getattr(&realpath, stat, request_mask, flags); if (err) goto out; @@ -306,7 +306,7 @@ int ovl_permission(struct mnt_idmap *idmap, if (err) return err; - guard(cred)(ovl_creds(inode->i_sb)); + cred_guard(ovl_creds(inode->i_sb)); if (!upperinode && !special_file(realinode->i_mode) && mask & MAY_WRITE) { @@ -328,7 +328,7 @@ static const char *ovl_get_link(struct dentry *dentry, if (!dentry) return ERR_PTR(-ECHILD); - guard(cred)(ovl_creds(inode->i_sb)); + cred_guard(ovl_creds(inode->i_sb)); p = vfs_get_link(ovl_dentry_real(dentry), done); return p; @@ -461,7 +461,7 @@ struct posix_acl *do_ovl_get_acl(struct mnt_idmap *idmap, acl = get_cached_acl_rcu(realinode, type); } else { - guard(cred)(ovl_creds(inode->i_sb)); + cred_guard(ovl_creds(inode->i_sb)); acl = ovl_get_acl_path(&realpath, posix_acl_xattr_name(type), noperm); } @@ -487,7 +487,7 @@ static int ovl_set_or_remove_acl(struct dentry *dentry, struct inode *inode, struct posix_acl *real_acl; ovl_path_lower(dentry, &realpath); - scoped_guard(cred, ovl_creds(dentry->d_sb)) + cred_scoped_guard(ovl_creds(dentry->d_sb)) real_acl = vfs_get_acl(mnt_idmap(realpath.mnt), realdentry, acl_name); @@ -510,7 +510,7 @@ static int ovl_set_or_remove_acl(struct dentry *dentry, struct inode *inode, if (err) goto out; - scoped_guard(cred, ovl_creds(dentry->d_sb)) { + cred_scoped_guard(ovl_creds(dentry->d_sb)) { if (acl) err = ovl_do_set_acl(ofs, realdentry, acl_name, acl); else @@ -589,7 +589,7 @@ static int ovl_fiemap(struct inode *inode, struct fiemap_extent_info *fieinfo, if (!realinode->i_op->fiemap) return -EOPNOTSUPP; - guard(cred)(ovl_creds(inode->i_sb)); + cred_guard(ovl_creds(inode->i_sb)); err = realinode->i_op->fiemap(realinode, fieinfo, start, len); return err; @@ -649,7 +649,7 @@ int ovl_fileattr_set(struct mnt_idmap *idmap, if (err) goto out; - guard(cred)(ovl_creds(inode->i_sb)); + cred_guard(ovl_creds(inode->i_sb)); /* * Store immutable/append-only flags in xattr and clear them * in upper fileattr (in case they were set by older kernel) @@ -717,7 +717,7 @@ int ovl_fileattr_get(struct dentry *dentry, struct fileattr *fa) ovl_path_real(dentry, &realpath); - guard(cred)(ovl_creds(inode->i_sb)); + cred_guard(ovl_creds(inode->i_sb)); err = ovl_real_fileattr_get(&realpath, fa); ovl_fileattr_prot_flags(inode, fa); diff --git a/fs/overlayfs/namei.c b/fs/overlayfs/namei.c index 4b9137572cdc..193f7015c8f5 100644 --- a/fs/overlayfs/namei.c +++ b/fs/overlayfs/namei.c @@ -953,7 +953,7 @@ static int ovl_maybe_validate_verity(struct dentry *dentry) return err; if (!ovl_test_flag(OVL_VERIFIED_DIGEST, inode)) { - guard(cred)(ovl_creds(dentry->d_sb)); + cred_guard(ovl_creds(dentry->d_sb)); err = ovl_validate_verity(ofs, &metapath, &datapath); if (err == 0) @@ -988,7 +988,7 @@ static int ovl_maybe_lookup_lowerdata(struct dentry *dentry) if (ovl_dentry_lowerdata(dentry)) goto out; - guard(cred)(ovl_creds(dentry->d_sb)); + cred_guard(ovl_creds(dentry->d_sb)); err = ovl_lookup_data_layers(dentry, redirect, &datapath); if (err) @@ -1055,7 +1055,7 @@ struct dentry *ovl_lookup(struct inode *dir, struct dentry *dentry, if (dentry->d_name.len > ofs->namelen) return ERR_PTR(-ENAMETOOLONG); - guard(cred)(ovl_creds(dentry->d_sb)); + cred_guard(ovl_creds(dentry->d_sb)); upperdir = ovl_dentry_upper(dentry->d_parent); if (upperdir) { d.layer = &ofs->layers[0]; @@ -1381,7 +1381,7 @@ bool ovl_lower_positive(struct dentry *dentry) if (!ovl_dentry_upper(dentry)) return true; - guard(cred)(ovl_creds(dentry->d_sb)); + cred_guard(ovl_creds(dentry->d_sb)); /* Positive upper -> have to look up lower to see whether it exists */ for (i = 0; !done && !positive && i < ovl_numlower(poe); i++) { struct dentry *this; diff --git a/fs/overlayfs/readdir.c b/fs/overlayfs/readdir.c index d17f8a5aae6f..41e01fe3ae4a 100644 --- a/fs/overlayfs/readdir.c +++ b/fs/overlayfs/readdir.c @@ -274,7 +274,7 @@ static int ovl_check_whiteouts(const struct path *path, struct ovl_readdir_data struct ovl_cache_entry *p; struct dentry *dentry, *dir = path->dentry; - guard(cred)(ovl_creds(rdd->dentry->d_sb)); + cred_guard(ovl_creds(rdd->dentry->d_sb)); err = down_write_killable(&dir->d_inode->i_rwsem); if (!err) { @@ -753,7 +753,7 @@ static int ovl_iterate(struct file *file, struct dir_context *ctx) struct ovl_cache_entry *p; int err; - guard(cred)(ovl_creds(dentry->d_sb)); + cred_guard(ovl_creds(dentry->d_sb)); if (!ctx->pos) ovl_dir_reset(file); @@ -853,7 +853,7 @@ static struct file *ovl_dir_open_realfile(const struct file *file, { struct file *res; - guard(cred)(ovl_creds(file_inode(file)->i_sb)); + cred_guard(ovl_creds(file_inode(file)->i_sb)); res = ovl_path_open(realpath, O_RDONLY | (file->f_flags & O_LARGEFILE)); return res; @@ -978,7 +978,7 @@ int ovl_check_empty_dir(struct dentry *dentry, struct list_head *list) struct ovl_cache_entry *p, *n; struct rb_root root = RB_ROOT; - guard(cred)(ovl_creds(dentry->d_sb)); + cred_guard(ovl_creds(dentry->d_sb)); err = ovl_dir_read_merged(dentry, list, &root); if (err) return err; diff --git a/fs/overlayfs/util.c b/fs/overlayfs/util.c index 4f237de0836c..fdce1e453d40 100644 --- a/fs/overlayfs/util.c +++ b/fs/overlayfs/util.c @@ -1169,7 +1169,7 @@ int ovl_nlink_start(struct dentry *dentry) if (d_is_dir(dentry) || !ovl_test_flag(OVL_INDEX, inode)) return 0; - guard(cred)(ovl_creds(dentry->d_sb)); + cred_guard(ovl_creds(dentry->d_sb)); /* * The overlay inode nlink should be incremented/decremented IFF the * upper operation succeeds, along with nlink change of upper inode. @@ -1198,7 +1198,7 @@ void ovl_nlink_end(struct dentry *dentry) ovl_drop_write(dentry); if (ovl_test_flag(OVL_INDEX, inode) && inode->i_nlink == 0) { - guard(cred)(ovl_creds(dentry->d_sb)); + cred_guard(ovl_creds(dentry->d_sb)); ovl_cleanup_index(dentry); } diff --git a/fs/overlayfs/xattrs.c b/fs/overlayfs/xattrs.c index cc4ab2d81162..ad1a656e2798 100644 --- a/fs/overlayfs/xattrs.c +++ b/fs/overlayfs/xattrs.c @@ -1,5 +1,6 @@ // SPDX-License-Identifier: GPL-2.0-only +#include <linux/cred.h> #include <linux/fs.h> #include <linux/xattr.h> #include "overlayfs.h" @@ -44,7 +45,7 @@ static int ovl_xattr_set(struct dentry *dentry, struct inode *inode, const char if (!value && !upperdentry) { ovl_path_lower(dentry, &realpath); - scoped_guard(cred, ovl_creds(dentry->d_sb)) + cred_scoped_guard(ovl_creds(dentry->d_sb)) err = vfs_getxattr(mnt_idmap(realpath.mnt), realdentry, name, NULL, 0); if (err < 0) goto out; @@ -62,7 +63,7 @@ static int ovl_xattr_set(struct dentry *dentry, struct inode *inode, const char if (err) goto out; - scoped_guard(cred, ovl_creds(dentry->d_sb)) { + cred_scoped_guard(ovl_creds(dentry->d_sb)) { if (value) { err = ovl_do_setxattr(ofs, realdentry, name, value, size, flags); @@ -86,7 +87,7 @@ static int ovl_xattr_get(struct dentry *dentry, struct inode *inode, const char struct path realpath; ovl_i_path_real(inode, &realpath); - guard(cred)(ovl_creds(dentry->d_sb)); + cred_guard(ovl_creds(dentry->d_sb)); res = vfs_getxattr(mnt_idmap(realpath.mnt), realpath.dentry, name, value, size); return res; } @@ -114,7 +115,7 @@ ssize_t ovl_listxattr(struct dentry *dentry, char *list, size_t size) char *s; size_t prefix_len, name_len; - scoped_guard(cred, ovl_creds(dentry->d_sb)) + cred_scoped_guard(ovl_creds(dentry->d_sb)) res = vfs_listxattr(realdentry, list, size); if (res <= 0 || size == 0) return res; diff --git a/include/linux/cred.h b/include/linux/cred.h index be1e211d82e0..5a532fce1713 100644 --- a/include/linux/cred.h +++ b/include/linux/cred.h @@ -171,7 +171,6 @@ static inline bool cap_ambient_invariant_ok(const struct cred *cred) cap_intersect(cred->cap_permitted, cred->cap_inheritable)); } - /* * Override creds without bumping reference count. Caller must ensure * reference remains valid or has taken reference. Almost always not the @@ -190,8 +189,12 @@ static inline void revert_creds_light(const struct cred *revert_cred) rcu_assign_pointer(current->cred, revert_cred); } -DEFINE_LOCK_GUARD_1(cred, const struct cred, _T->lock = override_creds_light(_T->lock), - revert_creds_light(_T->lock)); +DEFINE_LOCK_GUARD_1(cred, struct cred, + _T->lock = (struct cred *)override_creds_light(_T->lock), + revert_creds_light(_T->lock)); + +#define cred_guard(_cred) guard(cred)(((struct cred *)_cred)) +#define cred_scoped_guard(_cred) scoped_guard(cred, ((struct cred *)_cred)) /** * get_new_cred_many - Get references on a new set of credentials
Christian Brauner <brauner@kernel.org> writes: > On Thu, Feb 15, 2024 at 09:16:36PM -0800, Vinicius Costa Gomes wrote: >> Fix the following warning when defining a cleanup guard for a "const" >> pointer type: >> >> ./include/linux/cleanup.h:211:18: warning: return discards ‘const’ qualifier from pointer target type [-Wdiscarded-qualifiers] >> 211 | return _T->lock; \ >> | ~~^~~~~~ >> ./include/linux/cleanup.h:233:1: note: in expansion of macro ‘__DEFINE_UNLOCK_GUARD’ >> 233 | __DEFINE_UNLOCK_GUARD(_name, _type, _unlock, __VA_ARGS__) \ >> | ^~~~~~~~~~~~~~~~~~~~~ >> ./include/linux/cred.h:193:1: note: in expansion of macro ‘DEFINE_LOCK_GUARD_1’ >> 193 | DEFINE_LOCK_GUARD_1(cred, const struct cred, _T->lock = override_creds_light(_T->lock), >> | ^~~~~~~~~~~~~~~~~~~ >> >> Signed-off-by: Vinicius Costa Gomes <vinicius.gomes@intel.com> >> --- >> include/linux/cleanup.h | 2 +- >> 1 file changed, 1 insertion(+), 1 deletion(-) >> >> diff --git a/include/linux/cleanup.h b/include/linux/cleanup.h >> index c2d09bc4f976..085482ef46c8 100644 >> --- a/include/linux/cleanup.h >> +++ b/include/linux/cleanup.h >> @@ -208,7 +208,7 @@ static inline void class_##_name##_destructor(class_##_name##_t *_T) \ >> \ >> static inline void *class_##_name##_lock_ptr(class_##_name##_t *_T) \ >> { \ >> - return _T->lock; \ >> + return (void *)_T->lock; \ >> } > > I think both of these patches are a bit ugly as we burden the generic > cleanup code with casting to void which could cause actual issues. Fair point. > > Casting from const to non-const is rather specific to the cred code so I > would rather like to put the burden on the cred code instead of the > generic code if possible. For what it's worth, I liked your changes, will remove these two "fixes" from the series and use your suggestions in the next version. Thank you,
Christian Brauner <brauner@kernel.org> writes: > > So something like this? (Amir?) > > > -DEFINE_LOCK_GUARD_1(cred, const struct cred, _T->lock = override_creds_light(_T->lock), > - revert_creds_light(_T->lock)); > +DEFINE_LOCK_GUARD_1(cred, struct cred, > + _T->lock = (struct cred *)override_creds_light(_T->lock), > + revert_creds_light(_T->lock)); > + > +#define cred_guard(_cred) guard(cred)(((struct cred *)_cred)) > +#define cred_scoped_guard(_cred) scoped_guard(cred, ((struct cred *)_cred)) > > /** > * get_new_cred_many - Get references on a new set of credentials Thinking about proposing a PATCH version (with these suggestions applied), Amir has suggested in the past that I should propose two separate series: (1) introducing the guard helpers + backing file changes; (2) overlayfs changes; Any new ideas about this? Or should I go with this plan? Cheers,
On Mon, Mar 25, 2024 at 05:50:55PM -0700, Vinicius Costa Gomes wrote: > Christian Brauner <brauner@kernel.org> writes: > > > > > So something like this? (Amir?) > > > > > > -DEFINE_LOCK_GUARD_1(cred, const struct cred, _T->lock = override_creds_light(_T->lock), > > - revert_creds_light(_T->lock)); > > +DEFINE_LOCK_GUARD_1(cred, struct cred, > > + _T->lock = (struct cred *)override_creds_light(_T->lock), > > + revert_creds_light(_T->lock)); > > + > > +#define cred_guard(_cred) guard(cred)(((struct cred *)_cred)) > > +#define cred_scoped_guard(_cred) scoped_guard(cred, ((struct cred *)_cred)) > > > > /** > > * get_new_cred_many - Get references on a new set of credentials > > Thinking about proposing a PATCH version (with these suggestions applied), Amir > has suggested in the past that I should propose two separate series: > (1) introducing the guard helpers + backing file changes; > (2) overlayfs changes; > > Any new ideas about this? Or should I go with this plan? I mean make it two separate patches and I can provide Amir with a stable branch for the cleanup guards. I think that's what he wanted.
On Tue, Mar 26, 2024 at 11:53:12AM +0100, Christian Brauner wrote: > On Mon, Mar 25, 2024 at 05:50:55PM -0700, Vinicius Costa Gomes wrote: > > Christian Brauner <brauner@kernel.org> writes: > > > > > > > > So something like this? (Amir?) > > > > > > > > > -DEFINE_LOCK_GUARD_1(cred, const struct cred, _T->lock = override_creds_light(_T->lock), > > > - revert_creds_light(_T->lock)); > > > +DEFINE_LOCK_GUARD_1(cred, struct cred, > > > + _T->lock = (struct cred *)override_creds_light(_T->lock), > > > + revert_creds_light(_T->lock)); > > > + > > > +#define cred_guard(_cred) guard(cred)(((struct cred *)_cred)) > > > +#define cred_scoped_guard(_cred) scoped_guard(cred, ((struct cred *)_cred)) > > > > > > /** > > > * get_new_cred_many - Get references on a new set of credentials > > > > Thinking about proposing a PATCH version (with these suggestions applied), Amir > > has suggested in the past that I should propose two separate series: > > (1) introducing the guard helpers + backing file changes; > > (2) overlayfs changes; > > > > Any new ideas about this? Or should I go with this plan? > > I mean make it two separate patches and I can provide Amir with a stable > branch for the cleanup guards. I think that's what he wanted. But send them out in one series ofc. Amir and I can sort this if needed.
Christian Brauner <brauner@kernel.org> writes: > On Tue, Mar 26, 2024 at 11:53:12AM +0100, Christian Brauner wrote: >> On Mon, Mar 25, 2024 at 05:50:55PM -0700, Vinicius Costa Gomes wrote: >> > Christian Brauner <brauner@kernel.org> writes: >> > >> > > >> > > So something like this? (Amir?) >> > > >> > > >> > > -DEFINE_LOCK_GUARD_1(cred, const struct cred, _T->lock = override_creds_light(_T->lock), >> > > - revert_creds_light(_T->lock)); >> > > +DEFINE_LOCK_GUARD_1(cred, struct cred, >> > > + _T->lock = (struct cred *)override_creds_light(_T->lock), >> > > + revert_creds_light(_T->lock)); >> > > + >> > > +#define cred_guard(_cred) guard(cred)(((struct cred *)_cred)) >> > > +#define cred_scoped_guard(_cred) scoped_guard(cred, ((struct cred *)_cred)) >> > > >> > > /** >> > > * get_new_cred_many - Get references on a new set of credentials >> > >> > Thinking about proposing a PATCH version (with these suggestions applied), Amir >> > has suggested in the past that I should propose two separate series: >> > (1) introducing the guard helpers + backing file changes; >> > (2) overlayfs changes; >> > >> > Any new ideas about this? Or should I go with this plan? >> >> I mean make it two separate patches and I can provide Amir with a stable >> branch for the cleanup guards. I think that's what he wanted. > > But send them out in one series ofc. Amir and I can sort this if needed. Yeah, understood. Thank you,
diff --git a/include/linux/cleanup.h b/include/linux/cleanup.h index c2d09bc4f976..085482ef46c8 100644 --- a/include/linux/cleanup.h +++ b/include/linux/cleanup.h @@ -208,7 +208,7 @@ static inline void class_##_name##_destructor(class_##_name##_t *_T) \ \ static inline void *class_##_name##_lock_ptr(class_##_name##_t *_T) \ { \ - return _T->lock; \ + return (void *)_T->lock; \ }
Fix the following warning when defining a cleanup guard for a "const" pointer type: ./include/linux/cleanup.h:211:18: warning: return discards ‘const’ qualifier from pointer target type [-Wdiscarded-qualifiers] 211 | return _T->lock; \ | ~~^~~~~~ ./include/linux/cleanup.h:233:1: note: in expansion of macro ‘__DEFINE_UNLOCK_GUARD’ 233 | __DEFINE_UNLOCK_GUARD(_name, _type, _unlock, __VA_ARGS__) \ | ^~~~~~~~~~~~~~~~~~~~~ ./include/linux/cred.h:193:1: note: in expansion of macro ‘DEFINE_LOCK_GUARD_1’ 193 | DEFINE_LOCK_GUARD_1(cred, const struct cred, _T->lock = override_creds_light(_T->lock), | ^~~~~~~~~~~~~~~~~~~ Signed-off-by: Vinicius Costa Gomes <vinicius.gomes@intel.com> --- include/linux/cleanup.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)