linux-kernel - RE: [RFC PATCH 17/35] libceph, rbd: Use ceph

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: Windows password security audit tool. GUI, reports in PDF.

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

Message-ID: <8c5c734ee133a1a7cb5a3fcaa11db0e8614cae20.camel@ibm.com>
Date: Thu, 20 Mar 2025 21:45:37 +0000
From: Viacheslav Dubeyko <Slava.Dubeyko@....com>
To: David Howells <dhowells@...hat.com>
CC: "dongsheng.yang@...ystack.cn" <dongsheng.yang@...ystack.cn>,
        "linux-fsdevel@...r.kernel.org" <linux-fsdevel@...r.kernel.org>,
        "ceph-devel@...r.kernel.org" <ceph-devel@...r.kernel.org>,
        "slava@...eyko.com" <slava@...eyko.com>,
        "linux-kernel@...r.kernel.org"
	<linux-kernel@...r.kernel.org>,
        Alex Markuze <amarkuze@...hat.com>,
        "jlayton@...nel.org" <jlayton@...nel.org>,
        "idryomov@...il.com"
	<idryomov@...il.com>,
        "linux-block@...r.kernel.org"
	<linux-block@...r.kernel.org>
Subject: RE: [RFC PATCH 17/35] libceph, rbd: Use ceph_databuf encoding start/stop

On Tue, 2025-03-18 at 22:19 +0000, David Howells wrote:
> Viacheslav Dubeyko <Slava.Dubeyko@....com> wrote:
> 
> > > -		ceph_encode_string(&p, end, buf, len);
> > > +		BUG_ON(p + sizeof(__le32) + len > end);
> > 
> > Frankly speaking, it's hard to follow why sizeof(__le32) should be in the
> > equation. Maybe, it make sense to introduce some constant? The name of
> > constant makes understanding of this calculation more clear.
> 
> Look through the patch.  It's done all over the place, even on parts I haven't
> touched.  However, it's probably because of the way the string is encoded
> (4-byte LE length followed by the characters).
> 
> It probably would make sense to use a calculation wrapper for this.  I have
> this in fs/afs/yfsclient.c for example:
> 
> 	static size_t xdr_strlen(unsigned int len)
> 	{
> 		return sizeof(__be32) + round_up(len, sizeof(__be32));
> 	}
> 
> > > +	BUG_ON(sizeof(__le64) + sizeof(__le32) + wsize > req->request->front_alloc_len);
> > 
> > The same problem is here. It's hard to follow to this check by involving
> > sizeof(__le64) and sizeof(__le32) in calculation. What these numbers mean here?
> 
> Presumably the sizes of the protocol elements in the marshalled data.  If you
> want to clean all those up in some way, I can add your patch into my
> series;-).
> 

Yeah, I am considering to make the likewise cleanup. :)

Thanks,
Slava.