Hi, Dave,
Thanks for your review.
On 4/29/2015 3:43 PM, Dave Chinner wrote:
> On Wed, Apr 29, 2015 at 01:55:25AM +0000, Sheng Yong wrote:
>> commit 8275cdd0e7ac550dcce2b3ef6d2fb3b808c1ae59 upstream.
>>
>> Commit e461fcb ("xfs: remote attribute lookups require the value
>> length") passes the remote attribute length in the xfs_da_args
>> structure on lookup so that CRC calculations and validity checking
>> can be performed correctly by related code. This, unfortunately has
>> the side effect of changing the args->valuelen parameter in cases
>> where it shouldn't.
>>
>> That is, when we replace a remote attribute, the incoming
>> replacement stores the value and length in args->value and
>> args->valuelen, but then the lookup which finds the existing remote
>> attribute overwrites args->valuelen with the length of the remote
>> attribute being replaced. Hence when we go to create the new
>> attribute, we create it of the size of the existing remote
>> attribute, not the size it is supposed to be. When the new attribute
>> is much smaller than the old attribute, this results in a
>> transaction overrun and an ASSERT() failure on a debug kernel:
>>
>> XFS: Assertion failed: tp->t_blk_res_used <= tp->t_blk_res, file: fs/xfs/xfs_trans.c, line: 331
>>
>> Fix this by keeping the remote attribute value length separate to
>> the attribute value length in the xfs_da_args structure. The enables
>> us to pass the length of the remote attribute to be removed without
>> overwriting the new attribute's length.
>>
>> Also, ensure that when we save remote block contexts for a later
>> rename we zero the original state variables so that we don't confuse
>> the state of the attribute to be removes with the state of the new
>> attribute that we just added. [Spotted by Brain Foster.]
>>
>> Signed-off-by: Dave Chinner <dchinner@xxxxxxxxxx>
>> Reviewed-by: Brian Foster <bfoster@xxxxxxxxxx>
>> Signed-off-by: Dave Chinner <david@xxxxxxxxxxxxx>
>> [shengyong: backport to 3.10
>> - Addresse: CVE-2015-0274
>> - adjust context
>> - fs/xfs/xfs_attr_list.c comes from fs/xfs/xfs_attr.c and
>> fs/xfs/xfs_attr_leaf.c in linux 3.12]
>> Signed-off-by: Sheng Yong <shengyong1@xxxxxxxxxx>
>
> You are backporting to 3.10?
>
> Check when Commit e461fcb ("xfs: remote attribute lookups require
> the value length") was introduced:
You are right, commit e461fcb is not merged into 3.10, but as I mentioned in
the cover letter, commit 7ae077802 cherry-picked from e461fcb, and the cherry-pick
one is merged into 3.10 :-)
linux-stable$ git log --oneline 7ae077802 -n 1
7ae0778 xfs: remote attribute lookups require the value length
linux-stable$ git describe --contains 7ae077802
v3.10-rc3~5^2
thanks,
Sheng
>
> $ git describe --contains e461fcb
> for-linus-v3.11-rc1~53
> $
>
> Seems to me like the problem the CVE describes didn't exist in 3.10...
>
> Cheers,
>
> Dave.
>
--
To unsubscribe from this list: send the line "unsubscribe stable" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html