On Sat, 2019-06-08 at 16:57 +0300, Amir Goldstein wrote:
> Count struct files open RO together with inode reference count instead
> of using a dedicated i_readcount field. This will allow us to use the
> RO count also when CONFIG_IMA is not defined and will reduce the size of
> struct inode for 32bit archs when CONFIG_IMA is defined.
>
> We need this RO count for posix leases code, which currently naively
> checks i_count and d_count in an inaccurate manner.
>
> Should regular i_count overflow into RO count bias by struct files
> opened for write, it's not a big deal, as we mostly need the RO count
> to be reliable when the first writer comes along.
"i_count" has been defined forever. Has its meaning changed? This
patch implies that "i_readcount" was never really needed.
Mimi
>
> Cc: <stable@xxxxxxxxxxxxxxx> # v4.19
> Signed-off-by: Amir Goldstein <amir73il@xxxxxxxxx>
> ---
> include/linux/fs.h | 33 +++++++++++++++++++------------
> security/integrity/ima/ima_main.c | 2 +-
> 2 files changed, 21 insertions(+), 14 deletions(-)
>
> diff --git a/include/linux/fs.h b/include/linux/fs.h
> index f7fdfe93e25d..504bf17967dd 100644
> --- a/include/linux/fs.h
> +++ b/include/linux/fs.h
> @@ -694,9 +694,6 @@ struct inode {
> atomic_t i_count;
> atomic_t i_dio_count;
> atomic_t i_writecount;
> -#ifdef CONFIG_IMA
> - atomic_t i_readcount; /* struct files open RO */
> -#endif
> union {
> const struct file_operations *i_fop; /* former ->i_op->default_file_ops */
> void (*free_inode)(struct inode *);
> @@ -2890,26 +2887,36 @@ static inline bool inode_is_open_for_write(const struct inode *inode)
> return atomic_read(&inode->i_writecount) > 0;
> }
>
> -#ifdef CONFIG_IMA
> +/*
> + * Count struct files open RO together with inode rerefernce count.
> + * We need this count for IMA and for posix leases. The RO count should not
> + * include files opened RDWR nor files opened O_PATH and internal kernel
> + * inode references, like the ones taken by overlayfs and inotify.
> + * Should regular i_count overflow into I_RO_COUNT_BIAS by struct files
> + * opened for write, it's not a big deal, as we mostly need
> + * inode_is_open_rdonly() to be reliable when the first writer comes along.
> + */
> +#define I_RO_COUNT_SHIFT 10
> +#define I_RO_COUNT_BIAS (1UL << I_RO_COUNT_SHIFT)
> +
> static inline void i_readcount_dec(struct inode *inode)
> {
> - BUG_ON(!atomic_read(&inode->i_readcount));
> - atomic_dec(&inode->i_readcount);
> + WARN_ON(atomic_read(&inode->i_count) < I_RO_COUNT_BIAS);
> + atomic_sub(I_RO_COUNT_BIAS, &inode->i_count);
> }
> static inline void i_readcount_inc(struct inode *inode)
> {
> - atomic_inc(&inode->i_readcount);
> + atomic_add(I_RO_COUNT_BIAS, &inode->i_count);
> }
> -#else
> -static inline void i_readcount_dec(struct inode *inode)
> +static inline int i_readcount_read(const struct inode *inode)
> {
> - return;
> + return atomic_read(&inode->i_count) >> I_RO_COUNT_SHIFT;
> }
> -static inline void i_readcount_inc(struct inode *inode)
> +static inline bool inode_is_open_rdonly(const struct inode *inode)
> {
> - return;
> + return atomic_read(&inode->i_count) > I_RO_COUNT_BIAS;
> }
> -#endif
> +
> extern int do_pipe_flags(int *, int);
>
> #define __kernel_read_file_id(id) \
> diff --git a/security/integrity/ima/ima_main.c b/security/integrity/ima/ima_main.c
> index 357edd140c09..766bac778d11 100644
> --- a/security/integrity/ima/ima_main.c
> +++ b/security/integrity/ima/ima_main.c
> @@ -94,7 +94,7 @@ static void ima_rdwr_violation_check(struct file *file,
> bool send_tomtou = false, send_writers = false;
>
> if (mode & FMODE_WRITE) {
> - if (atomic_read(&inode->i_readcount) && IS_IMA(inode)) {
> + if (inode_is_open_rdonly(inode) && IS_IMA(inode)) {
> if (!iint)
> iint = integrity_iint_find(inode);
> /* IMA_MEASURE is set from reader side */
