* [bug report] ext4: Set flags on quota files directly
@ 2021-12-15 11:42 Dan Carpenter
2021-12-15 11:59 ` Jan Kara
0 siblings, 1 reply; 3+ messages in thread
From: Dan Carpenter @ 2021-12-15 11:42 UTC (permalink / raw)
To: jack; +Cc: linux-ext4
Hello Jan Kara,
The patch 957153fce8d2: "ext4: Set flags on quota files directly"
from Apr 6, 2017, leads to the following Smatch static checker
warning:
fs/ext4/super.c:6779 ext4_quota_on()
warn: missing error code here? 'IS_ERR()' failed. 'err' = '0'
fs/ext4/super.c
6761
6762 lockdep_set_quota_inode(path->dentry->d_inode, I_DATA_SEM_QUOTA);
6763 err = dquot_quota_on(sb, type, format_id, path);
6764 if (err) {
6765 lockdep_set_quota_inode(path->dentry->d_inode,
6766 I_DATA_SEM_NORMAL);
6767 } else {
6768 struct inode *inode = d_inode(path->dentry);
6769 handle_t *handle;
6770
6771 /*
6772 * Set inode flags to prevent userspace from messing with quota
6773 * files. If this fails, we return success anyway since quotas
6774 * are already enabled and this is not a hard failure.
6775 */
6776 inode_lock(inode);
6777 handle = ext4_journal_start(inode, EXT4_HT_QUOTA, 1);
6778 if (IS_ERR(handle))
--> 6779 goto unlock_inode;
This should set "err = PTR_ERR(handle)" right?
6780 EXT4_I(inode)->i_flags |= EXT4_NOATIME_FL | EXT4_IMMUTABLE_FL;
6781 inode_set_flags(inode, S_NOATIME | S_IMMUTABLE,
6782 S_NOATIME | S_IMMUTABLE);
6783 err = ext4_mark_inode_dirty(handle, inode);
6784 ext4_journal_stop(handle);
6785 unlock_inode:
6786 inode_unlock(inode);
6787 }
6788 return err;
6789 }
regards,
dan carpenter
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [bug report] ext4: Set flags on quota files directly
2021-12-15 11:42 [bug report] ext4: Set flags on quota files directly Dan Carpenter
@ 2021-12-15 11:59 ` Jan Kara
2021-12-15 12:05 ` Dan Carpenter
0 siblings, 1 reply; 3+ messages in thread
From: Jan Kara @ 2021-12-15 11:59 UTC (permalink / raw)
To: Dan Carpenter; +Cc: jack, linux-ext4
Hello Dan!
On Wed 15-12-21 14:42:31, Dan Carpenter wrote:
> The patch 957153fce8d2: "ext4: Set flags on quota files directly"
> from Apr 6, 2017, leads to the following Smatch static checker
> warning:
>
> fs/ext4/super.c:6779 ext4_quota_on()
> warn: missing error code here? 'IS_ERR()' failed. 'err' = '0'
>
> fs/ext4/super.c
> 6761
> 6762 lockdep_set_quota_inode(path->dentry->d_inode, I_DATA_SEM_QUOTA);
> 6763 err = dquot_quota_on(sb, type, format_id, path);
> 6764 if (err) {
> 6765 lockdep_set_quota_inode(path->dentry->d_inode,
> 6766 I_DATA_SEM_NORMAL);
> 6767 } else {
> 6768 struct inode *inode = d_inode(path->dentry);
> 6769 handle_t *handle;
> 6770
> 6771 /*
> 6772 * Set inode flags to prevent userspace from messing with quota
> 6773 * files. If this fails, we return success anyway since quotas
> 6774 * are already enabled and this is not a hard failure.
> 6775 */
> 6776 inode_lock(inode);
> 6777 handle = ext4_journal_start(inode, EXT4_HT_QUOTA, 1);
> 6778 if (IS_ERR(handle))
> --> 6779 goto unlock_inode;
>
> This should set "err = PTR_ERR(handle)" right?
The comment above explains it I guess. We don't want to return error if
ext4_journal_start() fails because it is a "soft" failure we can absorb.
Honza
--
Jan Kara <jack@suse.com>
SUSE Labs, CR
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [bug report] ext4: Set flags on quota files directly
2021-12-15 11:59 ` Jan Kara
@ 2021-12-15 12:05 ` Dan Carpenter
0 siblings, 0 replies; 3+ messages in thread
From: Dan Carpenter @ 2021-12-15 12:05 UTC (permalink / raw)
To: Jan Kara; +Cc: linux-ext4
On Wed, Dec 15, 2021 at 12:59:27PM +0100, Jan Kara wrote:
> Hello Dan!
>
> On Wed 15-12-21 14:42:31, Dan Carpenter wrote:
> > The patch 957153fce8d2: "ext4: Set flags on quota files directly"
> > from Apr 6, 2017, leads to the following Smatch static checker
> > warning:
> >
> > fs/ext4/super.c:6779 ext4_quota_on()
> > warn: missing error code here? 'IS_ERR()' failed. 'err' = '0'
> >
> > fs/ext4/super.c
> > 6761
> > 6762 lockdep_set_quota_inode(path->dentry->d_inode, I_DATA_SEM_QUOTA);
> > 6763 err = dquot_quota_on(sb, type, format_id, path);
> > 6764 if (err) {
> > 6765 lockdep_set_quota_inode(path->dentry->d_inode,
> > 6766 I_DATA_SEM_NORMAL);
> > 6767 } else {
> > 6768 struct inode *inode = d_inode(path->dentry);
> > 6769 handle_t *handle;
> > 6770
> > 6771 /*
> > 6772 * Set inode flags to prevent userspace from messing with quota
> > 6773 * files. If this fails, we return success anyway since quotas
> > 6774 * are already enabled and this is not a hard failure.
> > 6775 */
> > 6776 inode_lock(inode);
> > 6777 handle = ext4_journal_start(inode, EXT4_HT_QUOTA, 1);
> > 6778 if (IS_ERR(handle))
> > --> 6779 goto unlock_inode;
> >
> > This should set "err = PTR_ERR(handle)" right?
>
> The comment above explains it I guess. We don't want to return error if
> ext4_journal_start() fails because it is a "soft" failure we can absorb.
>
> Honza
Oh, yeah. Sorry.
regards,
dan carpenter
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2021-12-15 12:07 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2021-12-15 11:42 [bug report] ext4: Set flags on quota files directly Dan Carpenter
2021-12-15 11:59 ` Jan Kara
2021-12-15 12:05 ` Dan Carpenter
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).