On Tue, Feb 28, 2017 at 10:33:53AM +0100, Martin Pieuchot wrote:
> On 28/02/17(Tue) 07:15, Claudio Jeker wrote:
> > On Mon, Feb 27, 2017 at 10:22:03PM +0100, Alexander Bluhm wrote:
> > [...] 
> > > > +int
> > > > +pfkey_attach(struct socket *so, int proto)
> > > >  {
> > > 
> > > I think you forgot the check from pfkey_usrreq() here.
> > > 
> > >         if ((socket->so_proto->pr_protocol > PFKEY_PROTOCOL_MAX) ||
> > >             (socket->so_proto->pr_protocol < 0) ||
> > >             !pfkey_versions[socket->so_proto->pr_protocol])
> > >                 return (EPROTONOSUPPORT);
> > > 
> > 
> > Good catch. Something like that needs to be added.
> > Not sure if I should use proto or the socket->so_proto->pr_protocol...
> 
> Use proto when you can.  All the code manipulating ``socket'' will be
> soon audited to check if it needs some MP protection or not.  So less
> code to audit makes our lifes easier :)

This check is done because of the lookup in pfkey_versions[].

We have it in pfkey_attach(), pfkey_detach(), pfkey_output():
            pfkey_versions[socket->so_proto->pr_protocol]->create(socket)) != 0)
        rval = pfkey_versions[socket->so_proto->pr_protocol]->release(socket);
        error = pfkey_versions[socket->so_proto->pr_protocol]->send(socket,

So the value that is used as array index has to be checked.

If we want to change it, we should do it consistently and please
not in this large mechanical diff.

bluhm

Reply via email to