* "signed < sizeof()" bug in xfs_attr_shortform_verify() ?
@ 2020-08-25 21:10 Alexey Dobriyan
2020-08-25 22:18 ` Darrick J. Wong
0 siblings, 1 reply; 3+ messages in thread
From: Alexey Dobriyan @ 2020-08-25 21:10 UTC (permalink / raw)
To: linux-xfs; +Cc: darrick.wong
xfs_attr_shortform_verify() contains the following code:
int64_t size = ifp->if_bytes;
/*
* Give up if the attribute is way too short.
*/
if (size < sizeof(struct xfs_attr_sf_hdr))
return __this_address;
In general "if (signed < sizeof())" is wrong because of how type
promotions work. Such check won't catch small negative values.
I don't know XFS well enough to know if negative values were excluded
somewhere above the callchain, but maybe someone else does.
^ permalink raw reply [flat|nested] 3+ messages in thread* Re: "signed < sizeof()" bug in xfs_attr_shortform_verify() ?
2020-08-25 21:10 "signed < sizeof()" bug in xfs_attr_shortform_verify() ? Alexey Dobriyan
@ 2020-08-25 22:18 ` Darrick J. Wong
2020-08-27 8:15 ` Christoph Hellwig
0 siblings, 1 reply; 3+ messages in thread
From: Darrick J. Wong @ 2020-08-25 22:18 UTC (permalink / raw)
To: Alexey Dobriyan; +Cc: linux-xfs
On Wed, Aug 26, 2020 at 12:10:48AM +0300, Alexey Dobriyan wrote:
> xfs_attr_shortform_verify() contains the following code:
>
>
> int64_t size = ifp->if_bytes;
> /*
> * Give up if the attribute is way too short.
> */
> if (size < sizeof(struct xfs_attr_sf_hdr))
> return __this_address;
>
>
> In general "if (signed < sizeof())" is wrong because of how type
> promotions work. Such check won't catch small negative values.
>
> I don't know XFS well enough to know if negative values were excluded
> somewhere above the callchain, but maybe someone else does.
The initial allocations are always positive and the subsequent
xfs_idata_realloc are checked to prevent if_bytes from going negative,
but it does seem funny to me that if_bytes is declared int64_t...
--D
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: "signed < sizeof()" bug in xfs_attr_shortform_verify() ?
2020-08-25 22:18 ` Darrick J. Wong
@ 2020-08-27 8:15 ` Christoph Hellwig
0 siblings, 0 replies; 3+ messages in thread
From: Christoph Hellwig @ 2020-08-27 8:15 UTC (permalink / raw)
To: Darrick J. Wong; +Cc: Alexey Dobriyan, linux-xfs
On Tue, Aug 25, 2020 at 03:18:42PM -0700, Darrick J. Wong wrote:
> On Wed, Aug 26, 2020 at 12:10:48AM +0300, Alexey Dobriyan wrote:
> > xfs_attr_shortform_verify() contains the following code:
> >
> >
> > int64_t size = ifp->if_bytes;
> > /*
> > * Give up if the attribute is way too short.
> > */
> > if (size < sizeof(struct xfs_attr_sf_hdr))
> > return __this_address;
> >
> >
> > In general "if (signed < sizeof())" is wrong because of how type
> > promotions work. Such check won't catch small negative values.
> >
> > I don't know XFS well enough to know if negative values were excluded
> > somewhere above the callchain, but maybe someone else does.
>
> The initial allocations are always positive and the subsequent
> xfs_idata_realloc are checked to prevent if_bytes from going negative,
> but it does seem funny to me that if_bytes is declared int64_t...
Yes, the int64_t is weird. IIRC the signednes was done to simplify
some arithmertics in the reallocation case, but that shouldn't really
leak out..
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2020-08-27 8:15 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2020-08-25 21:10 "signed < sizeof()" bug in xfs_attr_shortform_verify() ? Alexey Dobriyan
2020-08-25 22:18 ` Darrick J. Wong
2020-08-27 8:15 ` Christoph Hellwig
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox