From 5d862cb0d9a4b6dcc97a88fa0d5a7a717566e5ab Mon Sep 17 00:00:00 2001 From: Tim Chase Date: Thu, 19 Dec 2013 00:30:56 -0600 Subject: [PATCH] Properly handle updates of variably-sized SA entries. 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 Signed-off-by: Ned Bass Signed-off-by: Brian Behlendorf Closes #1978 --- module/zfs/sa.c | 17 +++++++++++------ 1 file changed, 11 insertions(+), 6 deletions(-) diff --git a/module/zfs/sa.c b/module/zfs/sa.c index 13c09f92c707..fcc5f3ba03ac 100644 --- a/module/zfs/sa.c +++ b/module/zfs/sa.c @@ -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 *) @@ -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);