Re: bug report: xfrm: potential null deref in xfrm_bundle_lookup()

["Timo Teräs" <timo.teras@iki.fi>]

On 05/22/2010 11:24 PM, Dan Carpenter wrote:
> This is a smatch thing.  I couldn't tell if it was a real issue so I
> thought I would send this mail to the experts.  :)
> 
> net/xfrm/xfrm_policy.c +1679 xfrm_bundle_lookup(51)
> 	error: we previously assumed 'xdst' could be null.
>   1672          new_xdst = xfrm_resolve_and_create_bundle(pols, num_pols, fl, family, dst_orig);
>   1673          if (IS_ERR(new_xdst)) {
>   1674                  err = PTR_ERR(new_xdst);
>   1675                  if (err != -EAGAIN)
>   1676                          goto error;
>   1677                  if (oldflo == NULL)
>   1678                          goto make_dummy_bundle;
>   1679                  dst_hold(&xdst->u.dst);
>                                   ^^^^^^^^^^^
> 	Can xdst be NULL here?  It would have to be something like
> 	oldflo gets passed in as null and __xfrm_policy_lookup() fails.

No. xdst and oldflo point to same data structure, just to different
offset (and data type). If oldflo is not null, xdst is not either. See
their initialization around lines 1640. Since oldflo is explicitly
tested for not being null, xdst is valid too.

It might make this more obvious if we tested xdst for NULL instead of
oldflo, though.

- Timo