On Sun 25-12-22 21:28:43, Nathan Chancellor wrote:
> On Thu, Dec 22, 2022 at 11:16:00AM +0100, Jan Kara wrote:
> > When adding extent to a file fails, so far we've silently squelshed the
> > error. Make sure to propagate it up properly.
> >
> > Signed-off-by: Jan Kara <jack@xxxxxxx>
> > ---
> > fs/udf/inode.c | 41 +++++++++++++++++++++++++++--------------
> > 1 file changed, 27 insertions(+), 14 deletions(-)
> >
> > diff --git a/fs/udf/inode.c b/fs/udf/inode.c
> > index 09417342d8b6..15b3e529854b 100644
> > --- a/fs/udf/inode.c
> > +++ b/fs/udf/inode.c
> > @@ -57,15 +57,15 @@ static int udf_update_inode(struct inode *, int);
> > static int udf_sync_inode(struct inode *inode);
> > static int udf_alloc_i_data(struct inode *inode, size_t size);
> > static sector_t inode_getblk(struct inode *, sector_t, int *, int *);
> > -static int8_t udf_insert_aext(struct inode *, struct extent_position,
> > - struct kernel_lb_addr, uint32_t);
> > +static int udf_insert_aext(struct inode *, struct extent_position,
> > + struct kernel_lb_addr, uint32_t);
> > static void udf_split_extents(struct inode *, int *, int, udf_pblk_t,
> > struct kernel_long_ad *, int *);
> > static void udf_prealloc_extents(struct inode *, int, int,
> > struct kernel_long_ad *, int *);
> > static void udf_merge_extents(struct inode *, struct kernel_long_ad *, int *);
> > -static void udf_update_extents(struct inode *, struct kernel_long_ad *, int,
> > - int, struct extent_position *);
> > +static int udf_update_extents(struct inode *, struct kernel_long_ad *, int,
> > + int, struct extent_position *);
> > static int udf_get_block(struct inode *, sector_t, struct buffer_head *, int);
> >
> > static void __udf_clear_extent_cache(struct inode *inode)
> > @@ -795,7 +795,9 @@ static sector_t inode_getblk(struct inode *inode, sector_t block,
> > /* write back the new extents, inserting new extents if the new number
> > * of extents is greater than the old number, and deleting extents if
> > * the new number of extents is less than the old number */
> > - udf_update_extents(inode, laarr, startnum, endnum, &prev_epos);
> > + *err = udf_update_extents(inode, laarr, startnum, endnum, &prev_epos);
> > + if (*err < 0)
> > + goto out_free;
>
> This patch in next-20221226 as commit d8b39db5fab8 ("udf: Handle error when
> adding extent to a file") causes the following clang warning:
>
> fs/udf/inode.c:805:6: error: variable 'newblock' is used uninitialized whenever 'if' condition is true [-Werror,-Wsometimes-uninitialized]
> if (*err < 0)
> ^~~~~~~~
> fs/udf/inode.c:827:9: note: uninitialized use occurs here
> return newblock;
> ^~~~~~~~
> fs/udf/inode.c:805:2: note: remove the 'if' if its condition is always false
> if (*err < 0)
> ^~~~~~~~~~~~~
> fs/udf/inode.c:607:34: note: initialize the variable 'newblock' to silence this warning
> udf_pblk_t newblocknum, newblock;
> ^
> = 0
> 1 error generated.
Thanks for the report. It should be fixed now.
Honza
>
> > newblock = udf_get_pblock(inode->i_sb, newblocknum,
> > iinfo->i_location.partitionReferenceNum, 0);
> > @@ -1063,21 +1065,30 @@ static void udf_merge_extents(struct inode *inode, struct kernel_long_ad *laarr,
> > }
> > }
> >
> > -static void udf_update_extents(struct inode *inode, struct kernel_long_ad *laarr,
> > - int startnum, int endnum,
> > - struct extent_position *epos)
> > +static int udf_update_extents(struct inode *inode, struct kernel_long_ad *laarr,
> > + int startnum, int endnum,
> > + struct extent_position *epos)
> > {
> > int start = 0, i;
> > struct kernel_lb_addr tmploc;
> > uint32_t tmplen;
> > + int err;
> >
> > if (startnum > endnum) {
> > for (i = 0; i < (startnum - endnum); i++)
> > udf_delete_aext(inode, *epos);
> > } else if (startnum < endnum) {
> > for (i = 0; i < (endnum - startnum); i++) {
> > - udf_insert_aext(inode, *epos, laarr[i].extLocation,
> > - laarr[i].extLength);
> > + err = udf_insert_aext(inode, *epos,
> > + laarr[i].extLocation,
> > + laarr[i].extLength);
> > + /*
> > + * If we fail here, we are likely corrupting the extent
> > + * list and leaking blocks. At least stop early to
> > + * limit the damage.
> > + */
> > + if (err < 0)
> > + return err;
> > udf_next_aext(inode, epos, &laarr[i].extLocation,
> > &laarr[i].extLength, 1);
> > start++;
> > @@ -1089,6 +1100,7 @@ static void udf_update_extents(struct inode *inode, struct kernel_long_ad *laarr
> > udf_write_aext(inode, epos, &laarr[i].extLocation,
> > laarr[i].extLength, 1);
> > }
> > + return 0;
> > }
> >
> > struct buffer_head *udf_bread(struct inode *inode, udf_pblk_t block,
> > @@ -2107,12 +2119,13 @@ int8_t udf_current_aext(struct inode *inode, struct extent_position *epos,
> > return etype;
> > }
> >
> > -static int8_t udf_insert_aext(struct inode *inode, struct extent_position epos,
> > - struct kernel_lb_addr neloc, uint32_t nelen)
> > +static int udf_insert_aext(struct inode *inode, struct extent_position epos,
> > + struct kernel_lb_addr neloc, uint32_t nelen)
> > {
> > struct kernel_lb_addr oeloc;
> > uint32_t oelen;
> > int8_t etype;
> > + int err;
> >
> > if (epos.bh)
> > get_bh(epos.bh);
> > @@ -2122,10 +2135,10 @@ static int8_t udf_insert_aext(struct inode *inode, struct extent_position epos,
> > neloc = oeloc;
> > nelen = (etype << 30) | oelen;
> > }
> > - udf_add_aext(inode, &epos, &neloc, nelen, 1);
> > + err = udf_add_aext(inode, &epos, &neloc, nelen, 1);
> > brelse(epos.bh);
> >
> > - return (nelen >> 30);
> > + return err;
> > }
> >
> > int8_t udf_delete_aext(struct inode *inode, struct extent_position epos)
> > --
> > 2.35.3
> >
> >
--
Jan Kara <jack@xxxxxxxx>
SUSE Labs, CR
