{"action":"created","issue":{"url":"https://api.github.com/repos/openzfs/openzfs/issues/14","labels_url":"https://api.github.com/repos/openzfs/openzfs/issues/14/labels{/name}","comments_url":"https://api.github.com/repos/openzfs/openzfs/issues/14/comments","events_url":"https://api.github.com/repos/openzfs/openzfs/issues/14/events","html_url":"https://github.com/openzfs/openzfs/pull/14","id":112646316,"number":14,"title":"sa_find_sizes() may compute wrong SA header size","user":{"login":"behlendorf","id":148917,"avatar_url":"https://avatars.githubusercontent.com/u/148917?v=3","gravatar_id":"","url":"https://api.github.com/users/behlendorf","html_url":"https://github.com/behlendorf","followers_url":"https://api.github.com/users/behlendorf/followers","following_url":"https://api.github.com/users/behlendorf/following{/other_user}","gists_url":"https://api.github.com/users/behlendorf/gists{/gist_id}","starred_url":"https://api.github.com/users/behlendorf/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/behlendorf/subscriptions","organizations_url":"https://api.github.com/users/behlendorf/orgs","repos_url":"https://api.github.com/users/behlendorf/repos","events_url":"https://api.github.com/users/behlendorf/events{/privacy}","received_events_url":"https://api.github.com/users/behlendorf/received_events","type":"User","site_admin":false},"labels":[],"state":"open","locked":false,"assignee":null,"milestone":null,"comments":5,"created_at":"2015-10-21T18:03:50Z","updated_at":"2015-10-23T15:58:16Z","closed_at":null,"pull_request":{"url":"https://api.github.com/repos/openzfs/openzfs/pulls/14","html_url":"https://github.com/openzfs/openzfs/pull/14","diff_url":"https://github.com/openzfs/openzfs/pull/14.diff","patch_url":"https://github.com/openzfs/openzfs/pull/14.patch"},"body":"Under the right conditions sa_find_sizes() will compute an incorrect\r\nsize of the system attribute (SA) header.  This causes a failed assertion\r\nwhen the SA_HDR_SIZE_MATCH_LAYOUT() test returns false, and may lead\r\nto corruption of SA data.\r\n\r\nThe bug presents itself when there are more than two variable-length SAs\r\nof just the right size to fit in the bonus buffer of a dnode.  The\r\nexisting logic fails to account for the SA header space needed to store\r\nthe sizes of all the variable-length SAs.\r\n\r\nA reproducer was possible on Linux by setting the xattr=sa dataset\r\nproperty and storing xattrs on symbolic links.  Note the corrupt link\r\ntarget name:\r\n\r\n$ zfs set xattr=sa tank/fish\r\n$ cd /tank/fish\r\n$ ln -fs 12345678901234567 link\r\n$ setfattr -n trusted.0000000000000000000 -v 0x000000000000000000000000 -h link\r\n$ setfattr -n trusted.1111111111111111111 -v 0x000000000000000000000000 -h link\r\n$ ls -l link\r\nlrwxrwxrwx 1 root root 17 Dec  6 15:40 link -> 90123456701234567\r\n\r\nThe reference link below contains a reproducer for FreeBSD.\r\n\r\nReferences:\r\n  http://lists.open-zfs.org/pipermail/developer/2013-November/000306.html\r\n  https://github.com/zfsonlinux/zfs/issues/1890\r\n\r\nSifned-off-by: Ned Bass <bass6@llnl.gov>\r\nSigned-off-by: Brian Behlendorf <behlendorf1@llnl.gov>"},"comment":{"url":"https://api.github.com/repos/openzfs/openzfs/issues/comments/150617253","html_url":"https://github.com/openzfs/openzfs/pull/14#issuecomment-150617253","issue_url":"https://api.github.com/repos/openzfs/openzfs/issues/14","id":150617253,"user":{"login":"avg-I","id":1499700,"avatar_url":"https://avatars.githubusercontent.com/u/1499700?v=3","gravatar_id":"","url":"https://api.github.com/users/avg-I","html_url":"https://github.com/avg-I","followers_url":"https://api.github.com/users/avg-I/followers","following_url":"https://api.github.com/users/avg-I/following{/other_user}","gists_url":"https://api.github.com/users/avg-I/gists{/gist_id}","starred_url":"https://api.github.com/users/avg-I/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/avg-I/subscriptions","organizations_url":"https://api.github.com/users/avg-I/orgs","repos_url":"https://api.github.com/users/avg-I/repos","events_url":"https://api.github.com/users/avg-I/events{/privacy}","received_events_url":"https://api.github.com/users/avg-I/received_events","type":"User","site_admin":false},"created_at":"2015-10-23T15:58:16Z","updated_at":"2015-10-23T15:58:16Z","body":"I think that the problem fixed by this change is still present in FreeBSD as well as in illumos.\r\n\r\nThe fix looks good to me.  I think that the problem that it solves is that the current code stops adding to `hdrsize` once we get into the trailing `blkptr_t`-sized area. So, if we (almost) fill the bonus buffer but do not actually spill over, then the calculated header size could be less than it should be.\r\n\r\nP.S. another problem still present in illumos is the one fixed by the following ZoL and FreeBSD commits:\r\nzfsonlinux/zfs@5d862cb0d9a4b6dcc97a88fa0d5a7a717566e5ab\r\nfreebsd/freebsd@a7cf665d5f7e2a94346c692418677188cca0356f\r\nI think that ZoL commit message is much clearer while FreeBSD code is a little bit nicer :-)\r\nFusing those two commits together would be even better."}}