c/mirror_zfs
1
0
Fork 0
mirror of https://git.proxmox.com/git/mirror_zfs.git synced 2024-11-17 18:11:00 +03:00
Code Issues Packages Projects Releases Wiki Activity

Fix rounding discrepancy in sa_find_sizes()

Browse Source 
A rounding discrepancy exists between how sa_build_layouts() and
sa_find_sizes() calculate when the spill block needs to be kicked in.
This results in a narrow size range where sa_build_layouts() believes
there must be a spill block allocated but due to the discrepancy there
isn't.  A panic then occurs when the hdl->sa_spill NULL pointer is
dereferenced.

The following reproducer for this bug was isolated:

    truncate -s 128m /tmp/tank
    zpool create tank /tmp/tank
    zfs create -o xattr=sa tank/fish
    ln -s `perl -e 'print "z" x 41'` /tank/fish/z
    setfattr -hn trusted.foo -v`perl -e 'print "z"x45'` /tank/fish/z

This test results in roughly the following system attribute (SA)
layout:

  176 bytes - "standard" SA's
   41 bytes - name of symbolic link target
  100 bytes - XDR encoded nvlist for xattr
  ---
  317 bytes - total

Because 317 is less than DN_MAX_BONUSLEN (320), sa_find_sizes()
decides no spill block is needed. But sa_build_layouts() rounds 41 up
to 48 when computing the space requirements so it tries to switch to
the spill block.

Note that we were only able to reproduce this bug using a combination
of symbolic links and the Linux-specific xattr=sa dataset property.
So while this issue is not technically Linux-specific, it may be
difficult or impossible to hit the narrow size range needed to
reproduce it on other platforms.

To fix the discrepancy, round the running total in sa_find_sizes() up
to an 8-byte boundary before accounting for each SA, since this is how
they will be stored in the bonus and (possibly) spill buffers.

To make the intent of the code more clear, explicitly assert key
assumptions about expected alignment of data and whether spill-over
will occur.

Signed-off-by: Matthew Ahrens <mahrens@delphix.com
Signed-off-by: Brian Behlendorf <behlendorf1@llnl.gov>
Closes #1240
This commit is contained in:
Ned Bass 2013-01-29 15:49:15 -08:00 committed by Brian Behlendorf
parent 89103a2643
commit 67629d0f08
1 changed files with 5 additions and 0 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

5
module/zfs/sa.c
View File

@ -593,10 +593,12 @@ sa_find_sizes(sa_os_t *sa, sa_bulk_attr_t *attr_desc, int attr_count,
sizeof (sa_hdr_phys_t); sizeof (sa_hdr_phys_t);
full_space = (buftype == SA_BONUS) ? DN_MAX_BONUSLEN : db->db_size; full_space = (buftype == SA_BONUS) ? DN_MAX_BONUSLEN : db->db_size;
ASSERT(IS_P2ALIGNED(full_space, 8));
for (i = 0; i != attr_count; i++) { for (i = 0; i != attr_count; i++) {
boolean_t is_var_sz; boolean_t is_var_sz;
*total = P2ROUNDUP(*total, 8);
*total += attr_desc[i].sa_length; *total += attr_desc[i].sa_length;
if (done) if (done)
goto next; goto next;
@ -728,12 +730,15 @@ sa_build_layouts(sa_handle_t *hdl, sa_bulk_attr_t *attr_desc, int attr_count,
for (i = 0, len_idx = 0, hash = -1ULL; i != attr_count; i++) { for (i = 0, len_idx = 0, hash = -1ULL; i != attr_count; i++) {
uint16_t length; uint16_t length;
ASSERT(IS_P2ALIGNED(data_start, 8));
ASSERT(IS_P2ALIGNED(buf_space, 8));
attrs[i] = attr_desc[i].sa_attr; attrs[i] = attr_desc[i].sa_attr;
length = SA_REGISTERED_LEN(sa, attrs[i]); length = SA_REGISTERED_LEN(sa, attrs[i]);
if (length == 0) if (length == 0)
length = attr_desc[i].sa_length; length = attr_desc[i].sa_length;
if (buf_space < length) { /* switch to spill buffer */ if (buf_space < length) { /* switch to spill buffer */
VERIFY(spilling);
VERIFY(bonustype == DMU_OT_SA); VERIFY(bonustype == DMU_OT_SA);
if (buftype == SA_BONUS && !sa->sa_force_spill) { if (buftype == SA_BONUS && !sa->sa_force_spill) {
sa_find_layout(hdl->sa_os, hash, attrs_start, sa_find_layout(hdl->sa_os, hash, attrs_start,