Re: [PATCH 2/3] ceph: fix bounds check macros ceph_decode_need and ceph_encode_need

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]



On Wed, 18 Apr 2012, Alex Elder wrote:
> On 12/14/2011 02:24 PM, Xi Wang wrote:
> > Given a large n, the bounds check (*p + n>  end) can be bypassed due to
> > pointer wraparound.  A safer check is (n>  end - *p).
> > 
> > Signed-off-by: Xi Wang<xi.wang@xxxxxxxxx>
> 
> I noticed this proposed change never got committed.
> 
> It looks good, but I don't like the name "ceph_need()".
> 
> I am planning to pull this in soon, modified like this:
> 
> static inline int ceph_need_ok(void **p, void *end, size_t n)

Would something like ceph_has_room() make more sense?

> {
>        return end >= *p && n <= end - *p;
> }
> 
> And then used like this:
> 
>                if (!likely(ceph_need_ok(p, end, n)))
> 
> If you have an objection to that, please say so soon
> (and if you have no objection, please ACK).
> 
> Reviewed-by: Alex Elder <elder@xxxxxxxxxxxxx>
> 
> > ---
> >   include/linux/ceph/decode.h |    9 +++++++--
> >   1 files changed, 7 insertions(+), 2 deletions(-)
> > 
> > diff --git a/include/linux/ceph/decode.h b/include/linux/ceph/decode.h
> > index c5b6939..ea6db7b 100644
> > --- a/include/linux/ceph/decode.h
> > +++ b/include/linux/ceph/decode.h
> > @@ -12,6 +12,11 @@
> >    *   void *end    pointer to end of buffer (last byte + 1)
> >    */
> > 
> > +static inline int ceph_need(void **p, void *end, size_t n)
> > +{
> > +	return ((end<  *p) || (n>  end - *p));
> > +}
> > +
> >   static inline u64 ceph_decode_64(void **p)
> >   {
> >   	u64 v = get_unaligned_le64(*p);
> > @@ -47,7 +52,7 @@ static inline void ceph_decode_copy(void **p, void *pv,
> > size_t n)
> >    */
> >   #define ceph_decode_need(p, end, n, bad)		\
> >   	do {						\
> > -		if (unlikely(*(p) + (n)>  (end))) 	\
> > +		if (unlikely(ceph_need(p, end, n)))	\
> >   			goto bad;			\
> >   	} while (0)
> > 
> > @@ -166,7 +171,7 @@ static inline void ceph_encode_string(void **p, void
> > *end,
> > 
> >   #define ceph_encode_need(p, end, n, bad)		\
> >   	do {						\
> > -		if (unlikely(*(p) + (n)>  (end))) 	\
> > +		if (unlikely(ceph_need(p, end, n)))	\
> >   			goto bad;			\
> >   	} while (0)
> > 
> 
> --
> To unsubscribe from this list: send the line "unsubscribe ceph-devel" in
> the body of a message to majordomo@xxxxxxxxxxxxxxx
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 
> 
--
To unsubscribe from this list: send the line "unsubscribe ceph-devel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[CEPH Users]     [Information on CEPH]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Photo]     [Yosemite News]    [Yosemite Photos]    [Free Online Dating]     [Linux Kernel]     [Linux SCSI]     [XFree86]

Add to Google Powered by Linux