Skip to content

Commit

Permalink
UFS2: Fix DoS due to corrupted extattrfile
Browse files Browse the repository at this point in the history
Prior versions of FreeBSD (11.x) may have produced a corrupt extattr file.
(Specifically, r312416 accidentally fixed this defect by removing a strcpy.)
CURRENT FreeBSD supports disk images from those prior versions of FreeBSD.
Validate the internal structure as soon as we read it in from disk, to
prevent these extattr files from causing invariants violations and DoS.

Attempting to access the extattr portion of these files results in
EINTEGRITY.  At this time, the only way to repair files damaged in this way
is to copy the contents to another file and move it over the original.

PR:		244089
Reported by:	Andrea Venturoli <ml AT netfence.it>
Reviewed by:	kib
Discussed with:	mckusick (earlier draft)
Security:	no
Differential Revision:	https://reviews.freebsd.org/D27010
  • Loading branch information
cemeyer committed Oct 30, 2020
1 parent 3f38780 commit e679084
Show file tree
Hide file tree
Showing 2 changed files with 21 additions and 12 deletions.
31 changes: 20 additions & 11 deletions sys/ufs/ffs/ffs_vnops.c
Original file line number Diff line number Diff line change
Expand Up @@ -1200,9 +1200,8 @@ ffs_findextattr(u_char *ptr, u_int length, int nspace, const char *name,
eap = (struct extattr *)ptr;
eaend = (struct extattr *)(ptr + length);
for (; eap < eaend; eap = EXTATTR_NEXT(eap)) {
/* make sure this entry is complete */
if (EXTATTR_NEXT(eap) > eaend)
break;
KASSERT(EXTATTR_NEXT(eap) <= eaend,
("extattr next %p beyond %p", EXTATTR_NEXT(eap), eaend));
if (eap->ea_namespace != nspace || eap->ea_namelength != nlen
|| memcmp(eap->ea_name, name, nlen) != 0)
continue;
Expand All @@ -1216,8 +1215,9 @@ ffs_findextattr(u_char *ptr, u_int length, int nspace, const char *name,
}

static int
ffs_rdextattr(u_char **p, struct vnode *vp, struct thread *td, int extra)
ffs_rdextattr(u_char **p, struct vnode *vp, struct thread *td)
{
const struct extattr *eap, *eaend, *eapnext;
struct inode *ip;
struct ufs2_dinode *dp;
struct fs *fs;
Expand All @@ -1231,10 +1231,10 @@ ffs_rdextattr(u_char **p, struct vnode *vp, struct thread *td, int extra)
fs = ITOFS(ip);
dp = ip->i_din2;
easize = dp->di_extsize;
if ((uoff_t)easize + extra > UFS_NXADDR * fs->fs_bsize)
if ((uoff_t)easize > UFS_NXADDR * fs->fs_bsize)
return (EFBIG);

eae = malloc(easize + extra, M_TEMP, M_WAITOK);
eae = malloc(easize, M_TEMP, M_WAITOK);

liovec.iov_base = eae;
liovec.iov_len = easize;
Expand All @@ -1249,7 +1249,17 @@ ffs_rdextattr(u_char **p, struct vnode *vp, struct thread *td, int extra)
error = ffs_extread(vp, &luio, IO_EXT | IO_SYNC);
if (error) {
free(eae, M_TEMP);
return(error);
return (error);
}
/* Validate disk xattrfile contents. */
for (eap = (void *)eae, eaend = (void *)(eae + easize); eap < eaend;
eap = eapnext) {
eapnext = EXTATTR_NEXT(eap);
/* Bogusly short entry or bogusly long entry. */
if (eap->ea_length < sizeof(*eap) || eapnext > eaend) {
free(eae, M_TEMP);
return (EINTEGRITY);
}
}
*p = eae;
return (0);
Expand Down Expand Up @@ -1300,7 +1310,7 @@ ffs_open_ea(struct vnode *vp, struct ucred *cred, struct thread *td)
return (0);
}
dp = ip->i_din2;
error = ffs_rdextattr(&ip->i_ea_area, vp, td, 0);
error = ffs_rdextattr(&ip->i_ea_area, vp, td);
if (error) {
ffs_unlock_ea(vp);
return (error);
Expand Down Expand Up @@ -1606,9 +1616,8 @@ vop_listextattr {
eap = (struct extattr *)ip->i_ea_area;
eaend = (struct extattr *)(ip->i_ea_area + ip->i_ea_len);
for (; error == 0 && eap < eaend; eap = EXTATTR_NEXT(eap)) {
/* make sure this entry is complete */
if (EXTATTR_NEXT(eap) > eaend)
break;
KASSERT(EXTATTR_NEXT(eap) <= eaend,
("extattr next %p beyond %p", EXTATTR_NEXT(eap), eaend));
if (eap->ea_namespace != ap->a_attrnamespace)
continue;

Expand Down
2 changes: 1 addition & 1 deletion sys/ufs/ufs/extattr.h
Original file line number Diff line number Diff line change
Expand Up @@ -95,7 +95,7 @@ struct extattr {
* content referenced by eap.
*/
#define EXTATTR_NEXT(eap) \
((struct extattr *)(((u_char *)(eap)) + (eap)->ea_length))
((struct extattr *)(__DECONST(char *, (eap)) + (eap)->ea_length))
#define EXTATTR_CONTENT(eap) \
(void *)(((u_char *)(eap)) + EXTATTR_BASE_LENGTH(eap))
#define EXTATTR_CONTENT_SIZE(eap) \
Expand Down

0 comments on commit e679084

Please sign in to comment.