* re: ext4: convert file system to meta_bg if needed during resizing
@ 2020-02-29 0:39 Colin Ian King
2020-02-29 17:42 ` Theodore Y. Ts'o
0 siblings, 1 reply; 6+ messages in thread
From: Colin Ian King @ 2020-02-29 0:39 UTC (permalink / raw)
To: Theodore Ts'o, Andreas Dilger, Ext4 Developers List; +Cc: linux-kernel
Hi,
static analysis with Coverity has found an issue in function
ext4_convert_meta_bg() with the following commit
commit 1c6bd7173d66b3dfdefcedb38cabc1fb03997509
Author: Theodore Ts'o <tytso@mit.edu>
Date: Thu Sep 13 10:19:24 2012 -0400
ext4: convert file system to meta_bg if needed during resizing
The analysis is as follows:
1898
1899 errout:
1900 ret = ext4_journal_stop(handle);
1901 if (!err)
Unused value (UNUSED_VALUE)assigned_value: Assigning value from ret to
err here, but that stored value is not used.
1902 err = ret;
1903 return ret;
I'm not entirely sure why !err is being checked and then err is being
re-assigned and then is not used.
Colin
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: ext4: convert file system to meta_bg if needed during resizing
2020-02-29 0:39 ext4: convert file system to meta_bg if needed during resizing Colin Ian King
@ 2020-02-29 17:42 ` Theodore Y. Ts'o
2020-02-29 17:52 ` Colin Ian King
0 siblings, 1 reply; 6+ messages in thread
From: Theodore Y. Ts'o @ 2020-02-29 17:42 UTC (permalink / raw)
To: Colin Ian King; +Cc: Andreas Dilger, Ext4 Developers List, linux-kernel
On Sat, Feb 29, 2020 at 12:39:34AM +0000, Colin Ian King wrote:
> Hi,
>
> static analysis with Coverity has found an issue in function
> ext4_convert_meta_bg() with the following commit
>
> commit 1c6bd7173d66b3dfdefcedb38cabc1fb03997509
> Author: Theodore Ts'o <tytso@mit.edu>
> Date: Thu Sep 13 10:19:24 2012 -0400
>
> ext4: convert file system to meta_bg if needed during resizing
>
> The analysis is as follows:
>
> 1898
> 1899 errout:
> 1900 ret = ext4_journal_stop(handle);
> 1901 if (!err)
>
> Unused value (UNUSED_VALUE)assigned_value: Assigning value from ret to
> err here, but that stored value is not used.
>
> 1902 err = ret;
> 1903 return ret;
Line 1903 should be "return err".
Want to send a patch, or shall I just commit the fix?
- Ted
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: ext4: convert file system to meta_bg if needed during resizing
2020-02-29 17:42 ` Theodore Y. Ts'o
@ 2020-02-29 17:52 ` Colin Ian King
0 siblings, 0 replies; 6+ messages in thread
From: Colin Ian King @ 2020-02-29 17:52 UTC (permalink / raw)
To: Theodore Y. Ts'o; +Cc: Andreas Dilger, Ext4 Developers List, linux-kernel
On 29/02/2020 17:42, Theodore Y. Ts'o wrote:
> On Sat, Feb 29, 2020 at 12:39:34AM +0000, Colin Ian King wrote:
>> Hi,
>>
>> static analysis with Coverity has found an issue in function
>> ext4_convert_meta_bg() with the following commit
>>
>> commit 1c6bd7173d66b3dfdefcedb38cabc1fb03997509
>> Author: Theodore Ts'o <tytso@mit.edu>
>> Date: Thu Sep 13 10:19:24 2012 -0400
>>
>> ext4: convert file system to meta_bg if needed during resizing
>>
>> The analysis is as follows:
>>
>> 1898
>> 1899 errout:
>> 1900 ret = ext4_journal_stop(handle);
>> 1901 if (!err)
>>
>> Unused value (UNUSED_VALUE)assigned_value: Assigning value from ret to
>> err here, but that stored value is not used.
>>
>> 1902 err = ret;
>> 1903 return ret;
>
> Line 1903 should be "return err".
>
> Want to send a patch, or shall I just commit the fix?
>
> - Ted
>
I don't mind if you commit the fix, I wasn't sure what the appropriate
fix was, so I kudos to you Ted for that.
Colin
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: ext4: convert file system to meta_bg if needed during resizing
2012-09-18 11:46 Dan Carpenter
2012-09-18 11:52 ` Fengguang Wu
@ 2012-09-19 3:51 ` Theodore Ts'o
1 sibling, 0 replies; 6+ messages in thread
From: Theodore Ts'o @ 2012-09-19 3:51 UTC (permalink / raw)
To: Dan Carpenter; +Cc: linux-ext4, kernel-janitors, Fengguang Wu
On Tue, Sep 18, 2012 at 02:46:35PM +0300, Dan Carpenter wrote:
> 1775 struct ext4_inode_info *ei = 0;
> ^^^^^^
> Sparse is going to complain. Not sure why Fengguang hasn't emailed you.
> 1791 ei = EXT4_I(inode);
> ^^^^^^^^^^^^^^^^^^
> We only set "ei" if EXT4_FEATURE_COMPAT_RESIZE_INODE.
> ...
> 1829 nr = le32_to_cpu(ei->i_data[EXT4_DIND_BLOCK]);
> ^^^^^^^^^^
> Null deref. Perhaps inode implies EXT4_FEATURE_COMPAT_RESIZE_INODE?
Inode does imply EXT4_FEATURE_COMPAT_RESIZE_INODE, but I should make
the code cleaner.
Thanks for pointing this out.
- Ted
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: ext4: convert file system to meta_bg if needed during resizing
2012-09-18 11:46 Dan Carpenter
@ 2012-09-18 11:52 ` Fengguang Wu
2012-09-19 3:51 ` Theodore Ts'o
1 sibling, 0 replies; 6+ messages in thread
From: Fengguang Wu @ 2012-09-18 11:52 UTC (permalink / raw)
To: Dan Carpenter; +Cc: tytso, linux-ext4, kernel-janitors
On Tue, Sep 18, 2012 at 02:46:35PM +0300, Dan Carpenter wrote:
> Hello Theodore Ts'o,
>
> The patch 1c6bd7173d66: "ext4: convert file system to meta_bg if
> needed during resizing" from Sep 13, 2012, leads to the following
> warning:
> fs/ext4/resize.c:1829 ext4_convert_meta_bg()
> error: potential NULL dereference 'ei'.
>
> 1770 static int ext4_convert_meta_bg(struct super_block *sb, struct inode *inode)
> 1771 {
> 1772 handle_t *handle;
> 1773 struct ext4_sb_info *sbi = EXT4_SB(sb);
> 1774 struct ext4_super_block *es = sbi->s_es;
> 1775 struct ext4_inode_info *ei = 0;
> ^^^^^^
> Sparse is going to complain. Not sure why Fengguang hasn't emailed you.
Sorry I'm doing lots of changes these days and they apparently break
many things.. I'd expect the build test system to gradually restore to
normal in the coming days.
Thanks,
Fengguang
^ permalink raw reply [flat|nested] 6+ messages in thread
* re: ext4: convert file system to meta_bg if needed during resizing
@ 2012-09-18 11:46 Dan Carpenter
2012-09-18 11:52 ` Fengguang Wu
2012-09-19 3:51 ` Theodore Ts'o
0 siblings, 2 replies; 6+ messages in thread
From: Dan Carpenter @ 2012-09-18 11:46 UTC (permalink / raw)
To: tytso; +Cc: linux-ext4, kernel-janitors, Fengguang Wu
Hello Theodore Ts'o,
The patch 1c6bd7173d66: "ext4: convert file system to meta_bg if
needed during resizing" from Sep 13, 2012, leads to the following
warning:
fs/ext4/resize.c:1829 ext4_convert_meta_bg()
error: potential NULL dereference 'ei'.
1770 static int ext4_convert_meta_bg(struct super_block *sb, struct inode *inode)
1771 {
1772 handle_t *handle;
1773 struct ext4_sb_info *sbi = EXT4_SB(sb);
1774 struct ext4_super_block *es = sbi->s_es;
1775 struct ext4_inode_info *ei = 0;
^^^^^^
Sparse is going to complain. Not sure why Fengguang hasn't emailed you.
1776 ext4_fsblk_t nr;
1777 int i, ret, err = 0;
1778 int credits = 1;
1779
1780 ext4_msg(sb, KERN_INFO, "Converting file system to meta_bg");
1781 if (EXT4_HAS_COMPAT_FEATURE(sb, EXT4_FEATURE_COMPAT_RESIZE_INODE)) {
1782 if (es->s_reserved_gdt_blocks) {
1783 ext4_error(sb, "Unexpected non-zero "
1784 "s_reserved_gdt_blocks");
1785 return -EPERM;
1786 }
1787 if (!inode) {
1788 ext4_error(sb, "Unexpected NULL resize_inode");
1789 return -EPERM;
1790 }
1791 ei = EXT4_I(inode);
^^^^^^^^^^^^^^^^^^
We only set "ei" if EXT4_FEATURE_COMPAT_RESIZE_INODE.
1792
1793 /* Do a quick sanity check of the resize inode */
1794 if (inode->i_blocks != 1 << (inode->i_blkbits - 9))
1795 goto invalid_resize_inode;
1796 for (i = 0; i < EXT4_N_BLOCKS; i++) {
1797 if (i == EXT4_DIND_BLOCK) {
1798 if (ei->i_data[i])
1799 continue;
1800 else
1801 goto invalid_resize_inode;
1802 }
1803 if (ei->i_data[i])
1804 goto invalid_resize_inode;
1805 }
1806 credits += 3; /* block bitmap, bg descriptor, resize inode */
1807 }
1808
1809 handle = ext4_journal_start_sb(sb, credits);
1810 if (IS_ERR(handle))
1811 return PTR_ERR(handle);
1812
1813 err = ext4_journal_get_write_access(handle, sbi->s_sbh);
1814 if (err)
1815 goto errout;
1816
1817 EXT4_CLEAR_COMPAT_FEATURE(sb, EXT4_FEATURE_COMPAT_RESIZE_INODE);
1818 EXT4_SET_INCOMPAT_FEATURE(sb, EXT4_FEATURE_INCOMPAT_META_BG);
1819 sbi->s_es->s_first_meta_bg =
1820 cpu_to_le32(num_desc_blocks(sb, sbi->s_groups_count));
1821
1822 err = ext4_handle_dirty_super(handle, sb);
1823 if (err) {
1824 ext4_std_error(sb, err);
1825 goto errout;
1826 }
1827
1828 if (inode) {
1829 nr = le32_to_cpu(ei->i_data[EXT4_DIND_BLOCK]);
^^^^^^^^^^
Null deref. Perhaps inode implies EXT4_FEATURE_COMPAT_RESIZE_INODE?
1830 ext4_free_blocks(handle, inode, NULL, nr, 1,
1831 EXT4_FREE_BLOCKS_METADATA |
1832 EXT4_FREE_BLOCKS_FORGET);
1833 ei->i_data[EXT4_DIND_BLOCK] = 0;
1834 inode->i_blocks = 0;
1835
1836 err = ext4_mark_inode_dirty(handle, inode);
1837 if (err)
1838 ext4_std_error(sb, err);
1839 }
regards,
dan carpenter
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2020-02-29 17:52 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-02-29 0:39 ext4: convert file system to meta_bg if needed during resizing Colin Ian King
2020-02-29 17:42 ` Theodore Y. Ts'o
2020-02-29 17:52 ` Colin Ian King
-- strict thread matches above, loose matches on Subject: below --
2012-09-18 11:46 Dan Carpenter
2012-09-18 11:52 ` Fengguang Wu
2012-09-19 3:51 ` Theodore Ts'o
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).