* [PATCH 0/2] Fix write leases on overlayfs @ 2019-06-08 13:57 Amir Goldstein 2019-06-08 13:57 ` [PATCH 1/2] vfs: replace i_readcount with a biased i_count Amir Goldstein 2019-06-08 13:57 ` [PATCH 2/2] locks: eliminate false positive conflicts for write lease Amir Goldstein 0 siblings, 2 replies; 11+ messages in thread From: Amir Goldstein @ 2019-06-08 13:57 UTC (permalink / raw) To: Miklos Szeredi Cc: J . Bruce Fields, Jeff Layton, Mimi Zohar, Al Viro, linux-fsdevel, linux-unionfs, linux-integrity Miklos, This series fixes a v4.19 regression. It also provides correct semantics w.r.t RDONLY open counter that Bruce also needed for nfsd. I marked both patches for stable v4.19. I verified the changes using modified LTP fcntl tests [1], which I ran over xfs and overlayfs. Thanks, Amir. [1] https://github.com/amir73il/ltp/commits/overlayfs-devel Amir Goldstein (2): vfs: replace i_readcount with a biased i_count locks: eliminate false positive conflicts for write lease fs/locks.c | 26 ++++++++++++++---------- include/linux/fs.h | 33 +++++++++++++++++++------------ security/integrity/ima/ima_main.c | 2 +- 3 files changed, 37 insertions(+), 24 deletions(-) -- 2.17.1 ^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH 1/2] vfs: replace i_readcount with a biased i_count 2019-06-08 13:57 [PATCH 0/2] Fix write leases on overlayfs Amir Goldstein @ 2019-06-08 13:57 ` Amir Goldstein 2019-06-09 19:36 ` Miklos Szeredi ` (2 more replies) 2019-06-08 13:57 ` [PATCH 2/2] locks: eliminate false positive conflicts for write lease Amir Goldstein 1 sibling, 3 replies; 11+ messages in thread From: Amir Goldstein @ 2019-06-08 13:57 UTC (permalink / raw) To: Miklos Szeredi Cc: J . Bruce Fields, Jeff Layton, Mimi Zohar, Al Viro, linux-fsdevel, linux-unionfs, linux-integrity 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. Cc: <stable@vger.kernel.org> # v4.19 Signed-off-by: Amir Goldstein <amir73il@gmail.com> --- 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 */ -- 2.17.1 ^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [PATCH 1/2] vfs: replace i_readcount with a biased i_count 2019-06-08 13:57 ` [PATCH 1/2] vfs: replace i_readcount with a biased i_count Amir Goldstein @ 2019-06-09 19:36 ` Miklos Szeredi [not found] ` <20190610151836.5E2A3207E0@mail.kernel.org> 2019-06-12 12:51 ` Mimi Zohar 2 siblings, 0 replies; 11+ messages in thread From: Miklos Szeredi @ 2019-06-09 19:36 UTC (permalink / raw) To: Amir Goldstein Cc: J . Bruce Fields, Jeff Layton, Mimi Zohar, Al Viro, linux-fsdevel, overlayfs, linux-integrity On Sat, Jun 8, 2019 at 3:57 PM Amir Goldstein <amir73il@gmail.com> 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. > > Cc: <stable@vger.kernel.org> # v4.19 > Signed-off-by: Amir Goldstein <amir73il@gmail.com> > --- > 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. The bigger issue is allowing i_count to wrap around: all you need is a file with 1025 hard links and 4194304 opens of said file. Doable without privileges? Not sure, but it does seem pretty fragile. BTW the current 32 bit i_readcount that IMA uses also has wraparound issues, though that's not nearly as bad. Going to a 64 bit i_count would fix these, I guess. Thanks, Miklos ^ permalink raw reply [flat|nested] 11+ messages in thread
[parent not found: <20190610151836.5E2A3207E0@mail.kernel.org>]
* Re: [PATCH 1/2] vfs: replace i_readcount with a biased i_count [not found] ` <20190610151836.5E2A3207E0@mail.kernel.org> @ 2019-06-10 16:37 ` Amir Goldstein 0 siblings, 0 replies; 11+ messages in thread From: Amir Goldstein @ 2019-06-10 16:37 UTC (permalink / raw) To: Sasha Levin; +Cc: Miklos Szeredi, J . Bruce Fields, stable On Mon, Jun 10, 2019 at 6:18 PM Sasha Levin <sashal@kernel.org> wrote: > > Hi, > > [This is an automated email] > > This commit has been processed because it contains a -stable tag. > The stable tag indicates that it's relevant for the following trees: 4.19+ > > The bot has tested the following trees: v5.1.7, v4.19.48. > > v5.1.7: Failed to apply! Possible dependencies: > fdb0da89f4ba ("new inode method: ->free_inode()") > > v4.19.48: Failed to apply! Possible dependencies: > 1a16dbaf798c ("Document d_splice_alias() calling conventions for ->lookup() users.") > fdb0da89f4ba ("new inode method: ->free_inode()") > > > How should we proceed with this patch? > I will take care of backporting once patch is merged. Thanks, Amir. ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 1/2] vfs: replace i_readcount with a biased i_count 2019-06-08 13:57 ` [PATCH 1/2] vfs: replace i_readcount with a biased i_count Amir Goldstein 2019-06-09 19:36 ` Miklos Szeredi [not found] ` <20190610151836.5E2A3207E0@mail.kernel.org> @ 2019-06-12 12:51 ` Mimi Zohar 2019-06-12 15:09 ` Amir Goldstein 2 siblings, 1 reply; 11+ messages in thread From: Mimi Zohar @ 2019-06-12 12:51 UTC (permalink / raw) To: Amir Goldstein, Miklos Szeredi Cc: J . Bruce Fields, Jeff Layton, Al Viro, linux-fsdevel, linux-unionfs, linux-integrity 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@vger.kernel.org> # v4.19 > Signed-off-by: Amir Goldstein <amir73il@gmail.com> > --- > 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 */ ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 1/2] vfs: replace i_readcount with a biased i_count 2019-06-12 12:51 ` Mimi Zohar @ 2019-06-12 15:09 ` Amir Goldstein 2019-06-12 15:29 ` J . Bruce Fields 2019-06-12 15:59 ` Mimi Zohar 0 siblings, 2 replies; 11+ messages in thread From: Amir Goldstein @ 2019-06-12 15:09 UTC (permalink / raw) To: Mimi Zohar Cc: Miklos Szeredi, J . Bruce Fields, Jeff Layton, Al Viro, linux-fsdevel, overlayfs, linux-integrity On Wed, Jun 12, 2019 at 3:52 PM Mimi Zohar <zohar@linux.ibm.com> wrote: > > 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. > Not really. i_count is only used to know if object is referenced. It does not matter if user takes 1 or more references on i_count as long as user puts back all the references it took. If user took i_readcount, i_count cannot be zero, so short of overflow, we can describe i_readcount as a biased i_count. But if I am following Miklos' suggestion to make i_count 64bit, inode struct size is going to grow for 32bit arch when CONFIG_IMA is not defined, so to reduce impact, I will keep i_readcount as a separate member and let it be defined also when BITS_PER_LONG == 64 and implement inode_is_open_rdonly() using d_count and i_count when i_readcount is not defined. Let's see what people will have to say about that... Thanks, Amir. ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 1/2] vfs: replace i_readcount with a biased i_count 2019-06-12 15:09 ` Amir Goldstein @ 2019-06-12 15:29 ` J . Bruce Fields 2019-06-12 15:35 ` Amir Goldstein 2019-06-12 15:59 ` Mimi Zohar 1 sibling, 1 reply; 11+ messages in thread From: J . Bruce Fields @ 2019-06-12 15:29 UTC (permalink / raw) To: Amir Goldstein Cc: Mimi Zohar, Miklos Szeredi, Jeff Layton, Al Viro, linux-fsdevel, overlayfs, linux-integrity On Wed, Jun 12, 2019 at 06:09:59PM +0300, Amir Goldstein wrote: > But if I am following Miklos' suggestion to make i_count 64bit, inode > struct size is going to grow for 32bit arch when CONFIG_IMA is not > defined, so to reduce impact, I will keep i_readcount as a separate > member and let it be defined also when BITS_PER_LONG == 64 > and implement inode_is_open_rdonly() using d_count and i_count > when i_readcount is not defined. How bad would it be just to let the inode be a little bigger? How big is it already on 32 bit architectures? How much does this change e.g. how many inodes you can cache per megabyte? --b. ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 1/2] vfs: replace i_readcount with a biased i_count 2019-06-12 15:29 ` J . Bruce Fields @ 2019-06-12 15:35 ` Amir Goldstein 0 siblings, 0 replies; 11+ messages in thread From: Amir Goldstein @ 2019-06-12 15:35 UTC (permalink / raw) To: J . Bruce Fields Cc: Mimi Zohar, Miklos Szeredi, Jeff Layton, Al Viro, linux-fsdevel, overlayfs, linux-integrity On Wed, Jun 12, 2019 at 6:29 PM J . Bruce Fields <bfields@fieldses.org> wrote: > > On Wed, Jun 12, 2019 at 06:09:59PM +0300, Amir Goldstein wrote: > > But if I am following Miklos' suggestion to make i_count 64bit, inode > > struct size is going to grow for 32bit arch when CONFIG_IMA is not > > defined, so to reduce impact, I will keep i_readcount as a separate > > member and let it be defined also when BITS_PER_LONG == 64 > > and implement inode_is_open_rdonly() using d_count and i_count > > when i_readcount is not defined. > > How bad would it be just to let the inode be a little bigger? How big > is it already on 32 bit architectures? How much does this change e.g. > how many inodes you can cache per megabyte? > It's hard to answer how tiny changes like this impact users with different configs, especially to IoT ones, so I do not like increasing size of inode unconditionally, but I will go with: -#ifdef CONFIG_IMA +#if defined(CONFIG_IMA) || defined(CONFIG_FILE_LOCKING) atomic_t i_readcount; /* struct files open RO */ #endif So IoT guys can have an option to keep inode size the same and not let the locks code worry about it. OK? Thanks, Amir. ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 1/2] vfs: replace i_readcount with a biased i_count 2019-06-12 15:09 ` Amir Goldstein 2019-06-12 15:29 ` J . Bruce Fields @ 2019-06-12 15:59 ` Mimi Zohar 1 sibling, 0 replies; 11+ messages in thread From: Mimi Zohar @ 2019-06-12 15:59 UTC (permalink / raw) To: Amir Goldstein Cc: Miklos Szeredi, J . Bruce Fields, Jeff Layton, Al Viro, linux-fsdevel, overlayfs, linux-integrity On Wed, 2019-06-12 at 18:09 +0300, Amir Goldstein wrote: > On Wed, Jun 12, 2019 at 3:52 PM Mimi Zohar <zohar@linux.ibm.com> wrote: > > > > 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. > > > > Not really. > i_count is only used to know if object is referenced. > It does not matter if user takes 1 or more references on i_count > as long as user puts back all the references it took. > > If user took i_readcount, i_count cannot be zero, so short of overflow, > we can describe i_readcount as a biased i_count. Having a count was originally to make sure we weren't missing anything. As long as we can detect if a file is opened for read, the less IMA specific code there is, the better. > > But if I am following Miklos' suggestion to make i_count 64bit, inode > struct size is going to grow for 32bit arch when CONFIG_IMA is not > defined, so to reduce impact, I will keep i_readcount as a separate > member and let it be defined also when BITS_PER_LONG == 64 > and implement inode_is_open_rdonly() using d_count and i_count > when i_readcount is not defined. > > Let's see what people will have to say about that... Ok Mimi ^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH 2/2] locks: eliminate false positive conflicts for write lease 2019-06-08 13:57 [PATCH 0/2] Fix write leases on overlayfs Amir Goldstein 2019-06-08 13:57 ` [PATCH 1/2] vfs: replace i_readcount with a biased i_count Amir Goldstein @ 2019-06-08 13:57 ` Amir Goldstein [not found] ` <20190610151835.2FFC12089E@mail.kernel.org> 1 sibling, 1 reply; 11+ messages in thread From: Amir Goldstein @ 2019-06-08 13:57 UTC (permalink / raw) To: Miklos Szeredi Cc: J . Bruce Fields, Jeff Layton, Mimi Zohar, Al Viro, linux-fsdevel, linux-unionfs, linux-integrity check_conflicting_open() is checking for existing fd's open for read or for write before allowing to take a write lease. The check that was implemented using i_count and d_count is an approximation that has several false positives. For example, overlayfs since v4.19, takes an extra reference on the dentry; An open with O_PATH takes a reference on the inode and dentry although the file cannot be read nor written. Change the implementation to use inode_is_open_rdonly() and i_writecount to eliminate the false positive conflicts and allow a write lease to be taken on an overlayfs file. The change of behavior with existing fd's open with O_PATH is symmetric w.r.t. current behavior of lease breakers - an open with O_PATH currently does not break a write lease. Cc: <stable@vger.kernel.org> # v4.19 Signed-off-by: Amir Goldstein <amir73il@gmail.com> --- fs/locks.c | 26 ++++++++++++++++---------- 1 file changed, 16 insertions(+), 10 deletions(-) diff --git a/fs/locks.c b/fs/locks.c index ec1e4a5df629..4937cfdf611a 100644 --- a/fs/locks.c +++ b/fs/locks.c @@ -1753,10 +1753,10 @@ int fcntl_getlease(struct file *filp) } /** - * check_conflicting_open - see if the given dentry points to a file that has + * check_conflicting_open - see if the given file points to an inode that has * an existing open that would conflict with the * desired lease. - * @dentry: dentry to check + * @filp: file to check * @arg: type of lease that we're trying to acquire * @flags: current lock flags * @@ -1764,10 +1764,11 @@ int fcntl_getlease(struct file *filp) * conflict with the lease we're trying to set. */ static int -check_conflicting_open(const struct dentry *dentry, const long arg, int flags) +check_conflicting_open(struct file *filp, const long arg, int flags) { int ret = 0; - struct inode *inode = dentry->d_inode; + struct inode *inode = locks_inode(filp); + int self_wcount = 0, self_rcount = 0; if (flags & FL_LAYOUT) return 0; @@ -1775,8 +1776,14 @@ check_conflicting_open(const struct dentry *dentry, const long arg, int flags) if ((arg == F_RDLCK) && inode_is_open_for_write(inode)) return -EAGAIN; - if ((arg == F_WRLCK) && ((d_count(dentry) > 1) || - (atomic_read(&inode->i_count) > 1))) + /* Make sure that only read/write count is from lease requestor */ + if (filp->f_mode & FMODE_WRITE) + self_wcount = 1; + else if ((filp->f_mode & (FMODE_READ | FMODE_WRITE)) == FMODE_READ) + self_rcount = 1; + + if ((arg == F_WRLCK) && (i_readcount_read(inode) != self_rcount || + (atomic_read(&inode->i_writecount) != self_wcount))) ret = -EAGAIN; return ret; @@ -1786,8 +1793,7 @@ static int generic_add_lease(struct file *filp, long arg, struct file_lock **flp, void **priv) { struct file_lock *fl, *my_fl = NULL, *lease; - struct dentry *dentry = filp->f_path.dentry; - struct inode *inode = dentry->d_inode; + struct inode *inode = locks_inode(filp); struct file_lock_context *ctx; bool is_deleg = (*flp)->fl_flags & FL_DELEG; int error; @@ -1822,7 +1828,7 @@ generic_add_lease(struct file *filp, long arg, struct file_lock **flp, void **pr percpu_down_read(&file_rwsem); spin_lock(&ctx->flc_lock); time_out_leases(inode, &dispose); - error = check_conflicting_open(dentry, arg, lease->fl_flags); + error = check_conflicting_open(filp, arg, lease->fl_flags); if (error) goto out; @@ -1879,7 +1885,7 @@ generic_add_lease(struct file *filp, long arg, struct file_lock **flp, void **pr * precedes these checks. */ smp_mb(); - error = check_conflicting_open(dentry, arg, lease->fl_flags); + error = check_conflicting_open(filp, arg, lease->fl_flags); if (error) { locks_unlink_lock_ctx(lease); goto out; -- 2.17.1 ^ permalink raw reply related [flat|nested] 11+ messages in thread
[parent not found: <20190610151835.2FFC12089E@mail.kernel.org>]
* Re: [PATCH 2/2] locks: eliminate false positive conflicts for write lease [not found] ` <20190610151835.2FFC12089E@mail.kernel.org> @ 2019-06-10 16:38 ` Amir Goldstein 0 siblings, 0 replies; 11+ messages in thread From: Amir Goldstein @ 2019-06-10 16:38 UTC (permalink / raw) To: Sasha Levin; +Cc: Miklos Szeredi, J . Bruce Fields, stable On Mon, Jun 10, 2019 at 6:18 PM Sasha Levin <sashal@kernel.org> wrote: > > Hi, > > [This is an automated email] > > This commit has been processed because it contains a -stable tag. > The stable tag indicates that it's relevant for the following trees: 4.19+ > > The bot has tested the following trees: v5.1.7, v4.19.48. > > v5.1.7: Build failed! Errors: > fs/locks.c:1784:27: error: implicit declaration of function ‘i_readcount_read’; did you mean ‘i_readcount_dec’? [-Werror=implicit-function-declaration] > > v4.19.48: Failed to apply! Possible dependencies: > 7bbd1fc0e9f1 ("fs/locks: remove unnecessary white space.") > d6367d624137 ("fs/locks: use properly initialized file_lock when unlocking.") > > > How should we proceed with this patch? > I will take care of backporting once patch is merged. Thanks, Amir. ^ permalink raw reply [flat|nested] 11+ messages in thread
end of thread, other threads:[~2019-06-12 15:59 UTC | newest] Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2019-06-08 13:57 [PATCH 0/2] Fix write leases on overlayfs Amir Goldstein 2019-06-08 13:57 ` [PATCH 1/2] vfs: replace i_readcount with a biased i_count Amir Goldstein 2019-06-09 19:36 ` Miklos Szeredi [not found] ` <20190610151836.5E2A3207E0@mail.kernel.org> 2019-06-10 16:37 ` Amir Goldstein 2019-06-12 12:51 ` Mimi Zohar 2019-06-12 15:09 ` Amir Goldstein 2019-06-12 15:29 ` J . Bruce Fields 2019-06-12 15:35 ` Amir Goldstein 2019-06-12 15:59 ` Mimi Zohar 2019-06-08 13:57 ` [PATCH 2/2] locks: eliminate false positive conflicts for write lease Amir Goldstein [not found] ` <20190610151835.2FFC12089E@mail.kernel.org> 2019-06-10 16:38 ` Amir Goldstein
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.