Skip to content

Commit

Permalink
Properly handle updates of variably-sized SA entries.
Browse files Browse the repository at this point in the history
During the update process in sa_modify_attrs(), the sizes of existing
variably-sized SA entries are obtained from sa_lengths[]. The case where
a variably-sized SA was being replaced neglected to increment the index
into sa_lengths[], so subsequent variable-length SAs would be rewritten
with the wrong length. This patch adds the missing increment operation
so all variably-sized SA entries are stored with their correct lengths.

Previously, a size-changing update of a variably-sized SA that occurred
when there were other variably-sized SAs in the bonus buffer would cause
the subsequent SAs to be corrupted.  The most common case in which this
would occur is when a mode change caused the ZPL_DACL_ACES entry to
change size when a ZPL_DXATTR (SA xattr) entry already existed.

The following sequence would have caused a failure when xattr=sa was in
force and would corrupt the bonus buffer:

	open(filename, O_WRONLY | O_CREAT, 0600);
	...
	lsetxattr(filename, ...);	/* create xattr SA */
	chmod(filename, 0650);		/* enlarges the ACL */

Signed-off-by: Chris Dunlop <chris@onthe.net.au>
Signed-off-by: Ned Bass <bass6@llnl.gov>
Signed-off-by: Brian Behlendorf <behlendorf1@llnl.gov>
Closes #1978
  • Loading branch information
dweeezil authored and behlendorf committed Dec 20, 2013
1 parent ac03409 commit 5d862cb
Showing 1 changed file with 11 additions and 6 deletions.
17 changes: 11 additions & 6 deletions module/zfs/sa.c
Original file line number Diff line number Diff line change
Expand Up @@ -1732,25 +1732,30 @@ sa_modify_attrs(sa_handle_t *hdl, sa_attr_type_t newattr,
hdr = SA_GET_HDR(hdl, SA_BONUS);
idx_tab = SA_IDX_TAB_GET(hdl, SA_BONUS);
for (; k != 2; k++) {
/* iterate over each attribute in layout */
/*
* Iterate over each attribute in layout. Fetch the
* size of variable-length attributes needing rewrite
* from sa_lengths[].
*/
for (i = 0, length_idx = 0; i != count; i++) {
sa_attr_type_t attr;

attr = idx_tab->sa_layout->lot_attrs[i];
length = SA_REGISTERED_LEN(sa, attr);
if (attr == newattr) {
if (length == 0)
++length_idx;
if (action == SA_REMOVE) {
j++;
continue;
}
ASSERT(SA_REGISTERED_LEN(sa, attr) == 0);
ASSERT(length == 0);
ASSERT(action == SA_REPLACE);
SA_ADD_BULK_ATTR(attr_desc, j, attr,
locator, datastart, buflen);
} else {
length = SA_REGISTERED_LEN(sa, attr);
if (length == 0) {
if (length == 0)
length = hdr->sa_lengths[length_idx++];
}

SA_ADD_BULK_ATTR(attr_desc, j, attr,
NULL, (void *)
Expand All @@ -1772,7 +1777,7 @@ sa_modify_attrs(sa_handle_t *hdl, sa_attr_type_t newattr,
length = buflen;
}
SA_ADD_BULK_ATTR(attr_desc, j, newattr, locator,
datastart, buflen);
datastart, length);
}

error = sa_build_layouts(hdl, attr_desc, attr_count, tx);
Expand Down

0 comments on commit 5d862cb

Please sign in to comment.