Skip to content

Commit 10398ef

Browse files
andypriceAndreas Gruenbacher
authored andcommitted
gfs2: Improve gfs2_consist_inode() usage
gfs2_consist_inode() logs an error message with the source file and line number. When we jump before calling it, the line number becomes less useful as it no longer relates to the source of the error. To aid troubleshooting, replace the gotos with the gfs2_consist_inode() calls so that the error messages are more informative. Signed-off-by: Andrew Price <anprice@redhat.com> Signed-off-by: Andreas Gruenbacher <agruenba@redhat.com>
1 parent c95346a commit 10398ef

3 files changed

Lines changed: 53 additions & 40 deletions

File tree

fs/gfs2/dir.c

Lines changed: 17 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -562,15 +562,18 @@ static struct gfs2_dirent *gfs2_dirent_scan(struct inode *inode, void *buf,
562562
int ret = 0;
563563

564564
ret = gfs2_dirent_offset(GFS2_SB(inode), buf);
565-
if (ret < 0)
566-
goto consist_inode;
567-
565+
if (ret < 0) {
566+
gfs2_consist_inode(GFS2_I(inode));
567+
return ERR_PTR(-EIO);
568+
}
568569
offset = ret;
569570
prev = NULL;
570571
dent = buf + offset;
571572
size = be16_to_cpu(dent->de_rec_len);
572-
if (gfs2_check_dirent(GFS2_SB(inode), dent, offset, size, len, 1))
573-
goto consist_inode;
573+
if (gfs2_check_dirent(GFS2_SB(inode), dent, offset, size, len, 1)) {
574+
gfs2_consist_inode(GFS2_I(inode));
575+
return ERR_PTR(-EIO);
576+
}
574577
do {
575578
ret = scan(dent, name, opaque);
576579
if (ret)
@@ -582,8 +585,10 @@ static struct gfs2_dirent *gfs2_dirent_scan(struct inode *inode, void *buf,
582585
dent = buf + offset;
583586
size = be16_to_cpu(dent->de_rec_len);
584587
if (gfs2_check_dirent(GFS2_SB(inode), dent, offset, size,
585-
len, 0))
586-
goto consist_inode;
588+
len, 0)) {
589+
gfs2_consist_inode(GFS2_I(inode));
590+
return ERR_PTR(-EIO);
591+
}
587592
} while(1);
588593

589594
switch(ret) {
@@ -597,10 +602,6 @@ static struct gfs2_dirent *gfs2_dirent_scan(struct inode *inode, void *buf,
597602
BUG_ON(ret > 0);
598603
return ERR_PTR(ret);
599604
}
600-
601-
consist_inode:
602-
gfs2_consist_inode(GFS2_I(inode));
603-
return ERR_PTR(-EIO);
604605
}
605606

606607
static int dirent_check_reclen(struct gfs2_inode *dip,
@@ -609,14 +610,16 @@ static int dirent_check_reclen(struct gfs2_inode *dip,
609610
const void *ptr = d;
610611
u16 rec_len = be16_to_cpu(d->de_rec_len);
611612

612-
if (unlikely(rec_len < sizeof(struct gfs2_dirent)))
613-
goto broken;
613+
if (unlikely(rec_len < sizeof(struct gfs2_dirent))) {
614+
gfs2_consist_inode(dip);
615+
return -EIO;
616+
}
614617
ptr += rec_len;
615618
if (ptr < end_p)
616619
return rec_len;
617620
if (ptr == end_p)
618621
return -ENOENT;
619-
broken:
622+
620623
gfs2_consist_inode(dip);
621624
return -EIO;
622625
}

fs/gfs2/glops.c

Lines changed: 20 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -409,10 +409,14 @@ static int gfs2_dinode_in(struct gfs2_inode *ip, const void *buf)
409409
struct inode *inode = &ip->i_inode;
410410
bool is_new = inode->i_state & I_NEW;
411411

412-
if (unlikely(ip->i_no_addr != be64_to_cpu(str->di_num.no_addr)))
413-
goto corrupt;
414-
if (unlikely(!is_new && inode_wrong_type(inode, mode)))
415-
goto corrupt;
412+
if (unlikely(ip->i_no_addr != be64_to_cpu(str->di_num.no_addr))) {
413+
gfs2_consist_inode(ip);
414+
return -EIO;
415+
}
416+
if (unlikely(!is_new && inode_wrong_type(inode, mode))) {
417+
gfs2_consist_inode(ip);
418+
return -EIO;
419+
}
416420
ip->i_no_formal_ino = be64_to_cpu(str->di_num.no_formal_ino);
417421
inode->i_mode = mode;
418422
if (is_new) {
@@ -449,26 +453,28 @@ static int gfs2_dinode_in(struct gfs2_inode *ip, const void *buf)
449453
/* i_diskflags and i_eattr must be set before gfs2_set_inode_flags() */
450454
gfs2_set_inode_flags(inode);
451455
height = be16_to_cpu(str->di_height);
452-
if (unlikely(height > sdp->sd_max_height))
453-
goto corrupt;
456+
if (unlikely(height > sdp->sd_max_height)) {
457+
gfs2_consist_inode(ip);
458+
return -EIO;
459+
}
454460
ip->i_height = (u8)height;
455461

456462
depth = be16_to_cpu(str->di_depth);
457-
if (unlikely(depth > GFS2_DIR_MAX_DEPTH))
458-
goto corrupt;
463+
if (unlikely(depth > GFS2_DIR_MAX_DEPTH)) {
464+
gfs2_consist_inode(ip);
465+
return -EIO;
466+
}
459467
ip->i_depth = (u8)depth;
460468
ip->i_entries = be32_to_cpu(str->di_entries);
461469

462-
if (gfs2_is_stuffed(ip) && inode->i_size > gfs2_max_stuffed_size(ip))
463-
goto corrupt;
464-
470+
if (gfs2_is_stuffed(ip) && inode->i_size > gfs2_max_stuffed_size(ip)) {
471+
gfs2_consist_inode(ip);
472+
return -EIO;
473+
}
465474
if (S_ISREG(inode->i_mode))
466475
gfs2_set_aops(inode);
467476

468477
return 0;
469-
corrupt:
470-
gfs2_consist_inode(ip);
471-
return -EIO;
472478
}
473479

474480
/**

fs/gfs2/xattr.c

Lines changed: 16 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -96,30 +96,34 @@ static int ea_foreach_i(struct gfs2_inode *ip, struct buffer_head *bh,
9696
return -EIO;
9797

9898
for (ea = GFS2_EA_BH2FIRST(bh);; prev = ea, ea = GFS2_EA2NEXT(ea)) {
99-
if (!GFS2_EA_REC_LEN(ea))
100-
goto fail;
99+
if (!GFS2_EA_REC_LEN(ea)) {
100+
gfs2_consist_inode(ip);
101+
return -EIO;
102+
}
101103
if (!(bh->b_data <= (char *)ea && (char *)GFS2_EA2NEXT(ea) <=
102-
bh->b_data + bh->b_size))
103-
goto fail;
104-
if (!gfs2_eatype_valid(sdp, ea->ea_type))
105-
goto fail;
104+
bh->b_data + bh->b_size)) {
105+
gfs2_consist_inode(ip);
106+
return -EIO;
107+
}
108+
if (!gfs2_eatype_valid(sdp, ea->ea_type)) {
109+
gfs2_consist_inode(ip);
110+
return -EIO;
111+
}
106112
error = ea_call(ip, bh, ea, prev, data);
107113
if (error)
108114
return error;
109115

110116
if (GFS2_EA_IS_LAST(ea)) {
111117
if ((char *)GFS2_EA2NEXT(ea) !=
112-
bh->b_data + bh->b_size)
113-
goto fail;
118+
bh->b_data + bh->b_size) {
119+
gfs2_consist_inode(ip);
120+
return -EIO;
121+
}
114122
break;
115123
}
116124
}
117125

118126
return error;
119-
120-
fail:
121-
gfs2_consist_inode(ip);
122-
return -EIO;
123127
}
124128

125129
static int ea_foreach(struct gfs2_inode *ip, ea_call_t ea_call, void *data)

0 commit comments

Comments
 (0)