Archive-Team
/
zfs
2
0
Fork 0
Code Issues Pull Requests Packages Projects Releases 103 Wiki Activity

sa_find_sizes() may compute wrong SA header size 

Under the right conditions sa_find_sizes() will compute an incorrect
size of the system attribute (SA) header.  This causes a failed assertion
when the SA_HDR_SIZE_MATCH_LAYOUT() test returns false, and may lead
to corruption of SA data.

The bug presents itself when there are more than two variable-length SAs
of just the right size to fit in the bonus buffer of a dnode.  The
existing logic fails to account for the SA header space needed to store
the sizes of all the variable-length SAs.

A reproducer was possible on Linux by setting the xattr=sa dataset
property and storing xattrs on symbolic links (Issue #1648).  Note the
corrupt link target name:

$ zfs set xattr=sa tank/fish
$ cd /tank/fish
$ ln -fs 12345678901234567 link
$ setfattr -n trusted.0000000000000000000 -v 0x000000000000000000000000 -h link
$ setfattr -n trusted.1111111111111111111 -v 0x000000000000000000000000 -h link
$ ls -l link
lrwxrwxrwx 1 root root 17 Dec  6 15:40 link -> 90123456701234567

Commit 6a7c0ccca4 worked around this bug
by forcing xattr's on symlinks to be stored in directory format.  This
change implements a proper fix, so the workaround can now be reverted.

The reference link below contains a reproducer for FreeBSD.

References:
  http://lists.open-zfs.org/pipermail/developer/2013-November/000306.html

Ported-by: Ned Bass <bass6@llnl.gov>
Signed-off-by: Brian Behlendorf <behlendorf1@llnl.gov>
Closes #1890
This commit is contained in:
James Pan 2013-12-06 14:16:40 -08:00 committed by Brian Behlendorf
parent c1ab64d393
commit 472e7c6085
1 changed files with 21 additions and 24 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

45
module/zfs/sa.c
View File

@ -571,10 +571,9 @@ sa_find_sizes(sa_os_t *sa, sa_bulk_attr_t *attr_desc, int attr_count,
{ {
int var_size = 0; int var_size = 0;
int i; int i;
int j = -1;
int full_space; int full_space;
int hdrsize; int hdrsize;
boolean_t done = B_FALSE; int extra_hdrsize;
if (buftype == SA_BONUS && sa->sa_force_spill) { if (buftype == SA_BONUS && sa->sa_force_spill) {
*total = 0; *total = 0;
@ -585,10 +584,9 @@ sa_find_sizes(sa_os_t *sa, sa_bulk_attr_t *attr_desc, int attr_count,
*index = -1; *index = -1;
*total = 0; *total = 0;
*will_spill = B_FALSE;
if (buftype == SA_BONUS) extra_hdrsize = 0;
*will_spill = B_FALSE;
hdrsize = (SA_BONUSTYPE_FROM_DB(db) == DMU_OT_ZNODE) ? 0 : hdrsize = (SA_BONUSTYPE_FROM_DB(db) == DMU_OT_ZNODE) ? 0 :
sizeof (sa_hdr_phys_t); sizeof (sa_hdr_phys_t);
@ -600,8 +598,8 @@ sa_find_sizes(sa_os_t *sa, sa_bulk_attr_t *attr_desc, int attr_count,
*total = P2ROUNDUP(*total, 8); *total = P2ROUNDUP(*total, 8);
*total += attr_desc[i].sa_length; *total += attr_desc[i].sa_length;
if (done) if (*will_spill)
goto next; continue;
is_var_sz = (SA_REGISTERED_LEN(sa, attr_desc[i].sa_attr) == 0); is_var_sz = (SA_REGISTERED_LEN(sa, attr_desc[i].sa_attr) == 0);
if (is_var_sz) { if (is_var_sz) {
@ -609,21 +607,27 @@ sa_find_sizes(sa_os_t *sa, sa_bulk_attr_t *attr_desc, int attr_count,
} }
if (is_var_sz && var_size > 1) { if (is_var_sz && var_size > 1) {
if (P2ROUNDUP(hdrsize + sizeof (uint16_t), 8) + /* Don't worry that the spill block might overflow.
* It will be resized if needed in sa_build_layouts().
*/
if (buftype == SA_SPILL ||
P2ROUNDUP(hdrsize + sizeof (uint16_t), 8) +
*total < full_space) { *total < full_space) {
/* /*
* Account for header space used by array of * Account for header space used by array of
* optional sizes of variable-length attributes. * optional sizes of variable-length attributes.
* Record the index in case this increase needs * Record the extra header size in case this
* to be reversed due to spill-over. * increase needs to be reversed due to
* spill-over.
*/ */
hdrsize += sizeof (uint16_t); hdrsize += sizeof (uint16_t);
j = i; if (*index != -1)
extra_hdrsize += sizeof (uint16_t);
} else { } else {
done = B_TRUE; ASSERT(buftype == SA_BONUS);
*index = i; if (*index == -1)
if (buftype == SA_BONUS) *index = i;
*will_spill = B_TRUE; *will_spill = B_TRUE;
continue; continue;
} }
} }
@ -638,22 +642,15 @@ sa_find_sizes(sa_os_t *sa, sa_bulk_attr_t *attr_desc, int attr_count,
(*total + P2ROUNDUP(hdrsize, 8)) > (*total + P2ROUNDUP(hdrsize, 8)) >
(full_space - sizeof (blkptr_t))) { (full_space - sizeof (blkptr_t))) {
*index = i; *index = i;
done = B_TRUE;
} }
next:
if ((*total + P2ROUNDUP(hdrsize, 8)) > full_space && if ((*total + P2ROUNDUP(hdrsize, 8)) > full_space &&
buftype == SA_BONUS) buftype == SA_BONUS)
*will_spill = B_TRUE; *will_spill = B_TRUE;
} }
/* if (*will_spill)
* j holds the index of the last variable-sized attribute for hdrsize -= extra_hdrsize;
* which hdrsize was increased. Reverse the increase if that
* attribute will be relocated to the spill block.
*/
if (*will_spill && j == *index)
hdrsize -= sizeof (uint16_t);
hdrsize = P2ROUNDUP(hdrsize, 8); hdrsize = P2ROUNDUP(hdrsize, 8);
return (hdrsize); return (hdrsize);