linux-kernel - Re: [PATCH 1/1] XFS: xfs_iformat realtime device target pointer check

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-Id: <041FE945-8CBD-476C-83CD-E57748251711@sgi.com>
Date:	Wed, 5 Aug 2009 16:53:40 -0500
From:	Felix Blyakher <felixb@....com>
To:	Eric Sandeen <sandeen@...deen.net>
Cc:	Ramon de Carvalho Valle <ramon@...esecurity.org>,
	Christoph Hellwig <hch@...radead.org>,
	xfs mailing list <xfs@....sgi.com>,
	Christoph Hellwig <hch@....de>,
	Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
	mszeredi@...ell.com
Subject: Re: [PATCH 1/1] XFS: xfs_iformat realtime device target pointer check


On Aug 4, 2009, at 11:15 PM, Eric Sandeen wrote:

> Ramon de Carvalho Valle wrote:
>> On Tue, 2009-08-04 at 14:11 -0500, Eric Sandeen wrote:
>>> Ramon de Carvalho Valle wrote:
>>>> The xfs_iformat function does not check if the realtime device  
>>>> target pointer
>>>> is valid when the XFS_DIFLAG_REALTIME flag is set on the ondisk  
>>>> inode
>>>> structure.
>>>>
>>>> Signed-off-by: Christoph Hellwig <hch@...radead.org>
>>>> Signed-off-by: Ramon de Carvalho Valle <ramon@...esecurity.org>
>>>> Cc: stable <stable@...nel.org>
>>>> ---
>>>> fs/xfs/xfs_inode.c |   23 +++++++++++++++++------
>>>> 1 files changed, 17 insertions(+), 6 deletions(-)
>>>>
>>>> diff --git a/fs/xfs/xfs_inode.c b/fs/xfs/xfs_inode.c
>>>> index 1f22d65..37d3ee5 100644
>>>> --- a/fs/xfs/xfs_inode.c
>>>> +++ b/fs/xfs/xfs_inode.c
>>>> @@ -343,13 +343,24 @@ xfs_iformat(
>>>> 		return XFS_ERROR(EFSCORRUPTED);
>>>> 	}
>>>>
>>>> +	if (unlikely((ip->i_d.di_flags & XFS_DIFLAG_REALTIME) &&
>>>> +			!ip->i_mount->m_rtdev_targp)) {
>>>> +		xfs_fs_repair_cmn_err(CE_WARN, ip->i_mount,
>>>> +			"corrupt dinode %Lu, flags = 0x%x.",
>>>> +			(unsigned long long)ip->i_ino,
>>>> +			ip->i_d.di_flags);
>>>> +		XFS_CORRUPTION_ERROR("xfs_iformat(3)", XFS_ERRLEVEL_LOW,
>>>> +				     ip->i_mount, dip);
>>> I think I'd rather not change all the corruption text tag ordering;
>>> it'll make it harder to track down any common occurrences of
>>> "xfs_iformat(X)" corruption in the future if they get renumbered  
>>> now.
>>>
>>> I'd either make this xfs_iformat(2.1) ;) or just leave it as  
>>> Christoph
>>> had.  "realtime" is a lot more informative than "3" anyway.
>>
>> I don't think this is a bad decision, because the corruption errors  
>> can
>> be easily identified by the output of xfs_fs_repair_cmn_err and the
>> source line. I think this is a reasonable change that will keep the  
>> code
>> clean and consistent.
>
> Until you wind up looking at a problem from some old kernel, or  
> modified
> vendor kernel, and you realize that now you really don't know which
> error "xfs_iformat(6)" is anymore, and you either have to go digging
> through trees that aren't handy, or you just give up and don't  
> bother to
> help because now it's too much of a pain.  ;)
>
> But I can leave it up to the folks @ sgi, I can see both sides of the
> argument, and I won't care too much either way.

Agree with Eric, see the benefits of both approaches, but I think,
it'll be cleaner without shifting the numbering of all messages.
Otherwise, looks good.

Felix

>
>
> Thanks,
> -Eric
>
>> -Ramon
>>
>>> -Eric
>
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux- 
> kernel" in
> the body of a message to majordomo@...r.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/