tech-userlevel archive

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

Re: Reuse strtonum(3) and reallocarray(3) from OpenBSD



> Sent: Sunday, December 28, 2014 at 4:44 AM
> From: "Christos Zoulas" <christos%zoulas.com@localhost>
> To: "Kamil Rytarowski" <n54%gmx.com@localhost>
> Cc: tech-userlevel%netbsd.org@localhost, rmind%netbsd.org@localhost, "Joerg Sonnenberger" <joerg%britannica.bec.de@localhost>
> Subject: Re: Reuse strtonum(3) and reallocarray(3) from OpenBSD
>
> On Dec 28,  5:08am, n54%gmx.com@localhost ("Kamil Rytarowski") wrote:
> -- Subject: Re: Reuse strtonum(3) and reallocarray(3) from OpenBSD
> 
> | Your intention is to have a function that is not vulnerable to a mistakes
> | with not reseting errno to 0 before calling.
> 
> Yes. And not futzing with errno directly.

I see.

> 
> | That _r wrapper can be done around every of the strtol(3)-like functions.
> 
> We don't need to do all of them. Only the imax ones are useful.
> 

_r wrapper can be used in a private code of someone who wants to solve
shortcomings of C.

I would be upset seeing it at in libc... I was trying to find whether
there is some kind of strtol_r in wild... without much luck.

Useful? Please see below.

> | I understand your concerns, however I don't want this feature,
> | it's a design of C and I'm used to live with it to set errno to 0.
> | This is sad reality in cooperation with slimsy modules messing with errno.
> | Preserving errno won't solve any bugs, as other function calls are vulnerable as well.
> 
> We are talking about this particular function. And it is not that
> single line... To get errno, you need to include <errno.h>. If you
> don't care about errors, this is annoying.
> 

I see. Please see below.

> | This approach can hide real bugs.
> | 
> | An article from CERT:
> | 
> | "ERR30-C. Set errno to zero before calling a library function known to set errno, and check errno only after the function returns a value indicating failure"
> | https://www.securecoding.cert.org/confluence/pages/viewpage.action?pageId=6619179
> 
> Being overly aggressive about clearing errno can do that too.

It's not about being aggressive about clearing errno.

It's a good style of cooperating with external libs, for the same reason
it's a bad attitude to e.g. take mutex before calling an external function
from a slimsy module... we never know what can happen.

But if we control the external modules then with these integrity checks
the bugs pop up sooner.

> The
> original function of errno was not for detecting if an error occurred
> or not, but to find specific information about an error return from
> a function.

I miss this history in errno(2) (honestly).

> Setting errno and checking it violates that design.
> The reason CERT gives this advise is because of functions like
> strtol that violated that design (you cannot determine if an error
> occured by the functions return value).

Plato vs Aristoteles. Idealism vs realism.

What do you think about deal with estrtoi() and efullstrtoi(),
two new entries in efun(3)? I think it's a good compromise.

Like always, I'm willing to propose the full patch.


Home | Main Index | Thread Index | Old Index