ext4: Set flags on quota files directly
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
Hello Dan!
Quoted 26 lines · Dan Carpenter
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.c6761 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))
Quoted 3 lines
--> 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
Signature · 2 lines
Jan Kara <[email protected]> SUSE Labs, CR
Quoted 31 lines · Jan Kara
Hello Dan!
Quoted 28 lines · Dan Carpenter
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.c6761 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;
Quoted 6 lines
Quoted 1 line
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