Date: Fri, 08 Nov 2002 14:08:53 -0500 From: Brian Fundakowski Feldman <green@FreeBSD.org> To: current@FreeBSD.org Cc: phk@FreeBSD.org, mckusick@FreeBSD.org Subject: UFS2 extended attribute corruption woes Message-ID: <200211081908.gA8J8s06058917@green.bikeshed.org>
next in thread | raw e-mail | index | archive | help
I've been dealing for some weeks now with having my system unexpectedly lock
up and be unreachable via Ctrl-Alt-Esc, and tracked it down as far as "the
daily script" or "trying to relabel the entire filesystem" until recently.
Imagine my relief when rwatson suggests to try using the serial console
instead. I sez, "You're nuts! Why would the serial console make any
difference from the syscons for trying to break to the debugger? This is
definitely a hard lock-up," I sez to Robert.
Lo and behold, I try it, and in fact I _can_ break to the debugger over the
serial console! I find out that I'm in ffs_findextattr() and in an infinite
loop. Upon further investigation, I discover that (long story short) I have
three files on my system which have completely invalid extents: one has 48
bytes of gibberish (not just a few bytes being corrupt) and two have 48
bytes of 0s. I'm not going to wonder too hard about how they got there
since I had bad RAM in the laptop for a while, but I did go ahead and fix
FFS to be more resilient.
Here are my changes to just return EIO instead of looping infinitely. Does
it seem to be correct? I've also got a change to fsck in the trustedbsd_mac
which detects and dumps corrupt extents, but I don't have it clearing and
fixing them yet. I think both are probably important to have :)
==== //depot/projects/trustedbsd/mac/sys/ufs/ffs/ffs_vnops.c#15 (text+ko) - //depot/projects/trustedbsd/mac/sys/ufs/ffs/ffs_vnops.c#17 (text+ko) ==== content
@@ -1324,11 +1324,12 @@
* the length of the EA, and possibly the pointer to the entry and to the data.
*/
static int
-ffs_findextattr(u_char *ptr, uint length, int nspace, const char *name, u_char **eap, u_char **eac)
+ffs_findextattr(u_char *ptr, uint length, int nspace, const char *name,
+ int *ealenp, u_char **eap, u_char **eac)
{
u_char *p, *pe, *pn, *p0;
- int eapad1, eapad2, ealength, ealen, nlen;
- uint32_t ul;
+ int eapad1, ealength, ealen, nlen;
+ uint32_t ul, eapad2;
pe = ptr + length;
nlen = strlen(name);
@@ -1339,16 +1340,23 @@
pn = p + ul;
/* make sure this entry is complete */
if (pn > pe)
- break;
+ return (EIO);
+ /* don't loop forever on a corrupt entry */
+ if (pn <= p)
+ return (EIO);
p += sizeof(uint32_t);
if (*p != nspace)
continue;
p++;
eapad2 = *p++;
+ /* padding is at most 7 bytes */
+ if (eapad2 >= 8)
+ return (EIO);
if (*p != nlen)
continue;
p++;
- if (bcmp(p, name, nlen))
+ /* compare only up to the end of this attribute */
+ if (p + nlen > pn || bcmp(p, name, nlen))
continue;
ealength = sizeof(uint32_t) + 3 + nlen;
eapad1 = 8 - (ealength % 8);
@@ -1361,9 +1369,10 @@
*eap = p0;
if (eac != NULL)
*eac = p;
- return (ealen);
+ *ealenp = ealen;
+ return (0);
}
- return(-1);
+ return (ENOATTR);
}
static int
@@ -1597,16 +1606,13 @@
eae = ip->i_ea_area;
easize = ip->i_ea_len;
if (strlen(ap->a_name) > 0) {
- ealen = ffs_findextattr(eae, easize,
- ap->a_attrnamespace, ap->a_name, NULL, &p);
- if (ealen >= 0) {
- error = 0;
+ error = ffs_findextattr(eae, easize,
+ ap->a_attrnamespace, ap->a_name, &ealen, NULL, &p);
+ if (error == 0) {
if (ap->a_size != NULL)
*ap->a_size = ealen;
else if (ap->a_uio != NULL)
error = uiomove(p, ealen, ap->a_uio);
- } else {
- error = ENOATTR;
}
} else {
error = 0;
@@ -1711,16 +1717,16 @@
bcopy(ip->i_ea_area, eae, ip->i_ea_len);
easize = ip->i_ea_len;
- olen = ffs_findextattr(eae, easize,
- ap->a_attrnamespace, ap->a_name, &p, NULL);
- if (olen == -1 && ealength == 0) {
- /* delete but nonexistent */
+ error = ffs_findextattr(eae, easize,
+ ap->a_attrnamespace, ap->a_name, &olen, &p, NULL);
+ if ((error == ENOATTR && ealength == 0) || /* delete but nonexistent */
+ (error != 0 && error != ENOATTR)) { /* corrupted? */
free(eae, M_TEMP);
if (stand_alone)
ffs_close_ea(ap->a_vp, 0, ap->a_cred, ap->a_td);
- return(ENOATTR);
+ return (error);
}
- if (olen == -1) {
+ if (error == ENOATTR) {
/* new, append at end */
p = eae + easize;
easize += ealength;
--
Brian Fundakowski Feldman \'[ FreeBSD ]''''''''''\
<> green@FreeBSD.org <> bfeldman@tislabs.com \ The Power to Serve! \
Opinions expressed are my own. \,,,,,,,,,,,,,,,,,,,,,,\
To Unsubscribe: send mail to majordomo@FreeBSD.org
with "unsubscribe freebsd-current" in the body of the message
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?200211081908.gA8J8s06058917>