Re: [PATCH] loadpolicy: Verifies memory allocation during policy loading

["Daniel P. Smith" <dpsmith@xxxxxxxxxxxxxxxxxxxx>]

On 6/19/24 08:04, Jan Beulich wrote:
> On 27.05.2024 14:54, yskelg@xxxxxxxxx wrote:
> > --- a/tools/flask/utils/loadpolicy.c
> > +++ b/tools/flask/utils/loadpolicy.c
> > @@ -58,6 +58,11 @@ int main (int argCnt, const char *args[])
> >       }
> >   
> >       polMemCp = malloc(info.st_size);
> > +    if (!polMemCp) {
> > +        fprintf(stderr, "Error occurred allocating %ld bytes\n", info.st_size);
> > +        ret = -ENOMEM;
> I don't think -ENOMEM is valid to use here. See neighboring code. Nevertheless
> it is correct that a check should be here.
>
> As to %ld - is that portably usable with an off_t value?
>
> In any event, Daniel, really your turn to review / ack. I'm looking at this
> merely because I found this and another bugfix still sit in waiting-for-ack
> state.
I saw this but was on the fence of whether it really required my ack 
since it was more of a toolstack code fix versus an XSM relevant change.
With that said, and to expand on Jan's comment regarding ENOMEM, the 
utility does not currently differentiate main's return code. Unless the 
tools maintainer wants to start changing this, I would suggest setting 
ret to -1.
As to the '%ld', aligning with Jan's first comment, perhaps you might 
consider just reporting `strerror(errno)` similar to the other error 
handling checks. NB: it is likely errno will be set to -ENOMEM, so by 
doing this you will end up notifying ENOMEM occurred as you were 
attempting to do by providing it with `ret`. Additionally, then you 
won't have to deal with portability concerns over off_t.
V/r,
Daniel P. Smith