linux-kernel - Re: [PATCH] RDMA/ocrdma: Fix an off-by-one issue in 'ocrdma_add

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]

Date:   Fri, 17 Apr 2020 09:25:42 -0300
From:   Jason Gunthorpe <jgg@...pe.ca>
To:     Dan Carpenter <dan.carpenter@...cle.com>, g@...pe.ca
Cc:     Christophe JAILLET <christophe.jaillet@...adoo.fr>,
        selvin.xavier@...adcom.com, devesh.sharma@...adcom.com,
        dledford@...hat.com, leon@...nel.org, colin.king@...onical.com,
        linux-rdma@...r.kernel.org, linux-kernel@...r.kernel.org,
        kernel-janitors@...r.kernel.org
Subject: Re: [PATCH] RDMA/ocrdma: Fix an off-by-one issue in 'ocrdma_add_stat'

On Fri, Apr 17, 2020 at 02:26:24PM +0300, Dan Carpenter wrote:
> On Thu, Apr 16, 2020 at 03:47:54PM -0300, Jason Gunthorpe wrote:
> > On Thu, Apr 16, 2020 at 04:08:47PM +0300, Dan Carpenter wrote:
> > > On Tue, Apr 14, 2020 at 03:34:41PM -0300, Jason Gunthorpe wrote:
> > > > The memcpy is still kind of silly right? What about this:
> > > > 
> > > > static int ocrdma_add_stat(char *start, char *pcur, char *name, u64 count)
> > > > {
> > > > 	size_t len = (start + OCRDMA_MAX_DBGFS_MEM) - pcur;
> > > > 	int cpy_len;
> > > > 
> > > > 	cpy_len = snprintf(pcur, len, "%s: %llu\n", name, count);
> > > > 	if (cpy_len >= len || cpy_len < 0) {
> > > 
> > > The kernel version of snprintf() doesn't and will never return
> > > negatives.  It would cause a huge security headache if it started
> > > returning negatives.
> > 
> > Begs the question why it returns an int then :)
> 
> People should use "int" as their default type.  "int i;".  It means
> "This is a normal number.  Nothing special about it.  It's not too high.
> It's not defined by hardware requirements."  Other types call attention
> to themselves, but int is the humble datatype.

No, I strongly disagree with this, it is one of my pet peeves to see
'int' being used for data which is known to be only ever be positive
just to save typing 'unsigned'.

Not only is it confusing, but allowing signed values has caused tricky
security bugs, unfortuntely.

Jason