Re: Possible use-after-free in transport.c:SendReceive and SendReceive2

[Jeff Layton <jlayton@xxxxxxxxx>]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

On Fri, 10 Aug 2012 12:36:10 -0700
Richard Sharpe <realrichardsharpe@xxxxxxxxx> wrote:

> Hi folks,
> 
> I have been trawling through the code to get a better understanding of
> various things and I came across this curious thing in the smb2dev
> branch in git://git.altlinux.org/people/piastry/public/cifs-2.6.git
> but it is also in the master branch, it seems.
> 
> In transport.c:SendReceive (and SendReceive2 where we do similar things) we see:
> 
> ...
>         rc = cifs_sync_mid_result(midQ, ses->server);
>         if (rc != 0) {
>                 add_credits(ses->server, 1, 0);
>                 return rc;
>         }
> 

Note that here we return rc here without "goto out;" whenever rc != 0.

>         if (!midQ->resp_buf || !out_buf ||
>             midQ->mid_state != MID_RESPONSE_RECEIVED) {
>                 rc = -EIO;
>                 cERROR(1, "Bad MID state?");
>                 goto out;
>         }
> 
>         *pbytes_returned = get_rfc1002_length(midQ->resp_buf);
>         memcpy(out_buf, midQ->resp_buf, *pbytes_returned + 4);
>         rc = cifs_check_receive(midQ, ses->server, 0);
> ...
> 
> However, at the end of cifs_sync_mid_result we see:
> 
> 
>         DeleteMidQEntry(mid);
>         return rc;
> 
> and there are no early returns from cifs_sync_mid_result, it seems.
> The "mid" in the DeleteMidQEntry call is the same midQ passed to
> cifs_sync_mid_result, I believe.
> 

There is an early return in the MID_RESPONSE_RECEIVED case.

> So, it looks like we have a use-after-free situation and thus a race
> to get the data out before someone else pollutes it.
> 
> Is this correct or have I got the wrong end of the stick here?
> 

I don't think there is a use-after-free here, but I'll freely admit
that this code could be clearer. If you care to clean it up and make it
clearer, I certainly wouldn't object.

- -- 
Jeff Layton <jlayton@xxxxxxxxx>
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v2.0.18 (GNU/Linux)

iQIcBAEBAgAGBQJQJbVxAAoJEAAOaEEZVoIVWCgP/3h9HJkRDtGWDSVxYEIFF/n/
9VpMvFj6n7wjRo87gMi69t/ouJyRBfj0ytKJ10fSCN0DONgf2ruk8xMpDNVTGFOB
oSM/9hill/x54tUz+luhJ42cO+ELZ7bskW6jFWB9uQ8uvsI3wkeD3RuIQlniS1vs
3/0eHsRdiCUuavn48pzjESWhIKtvXq5RvLju2Abb+oKmZ1T6rNAKtIAfF0UWLzug
SOlLtw+o9F+j0Iu7nC0j152PwToK2OabT/CWTMbfF0gDnpuaJmVCXWSexp80OAYS
UF1kkBwgk/1H5o9pbDmY99o9TXiopk3MmvZAqstol+ND4NFhYbBpDZc/whLnyDvb
1Memp20nuIhVh1/KetmGB45CtUe5HIsJw1KdjMxXM4uWuUlPMAqfHIc3zeZnKE7C
j9wwJQBjMAmZwIlFwLGKV92dR5UMIH5ORGCZn0uKySPSwUFMvTUDK0fy/F7L8y+q
tPVd3gwWrfBKNIdPSmwF8B/OIsqdYKGZ1cGfEUlm2NHjI5z/lA/orHPE0DegDWtd
dQ6r6W9MP274xpbWHzldBJQcozwZNSrtSGrgIUytf3NPHJThzzwamVhQZY5u5ZzQ
BE5ZTJQz8dVZMgbeCO4qQAICUz4FDVEvrF9gZZ48cbPubSS6fKyTNTnxfUo0Gce3
hmVgx73LP2ZyuXv4Zaus
=SXOl
-----END PGP SIGNATURE-----
ÿôèº{.nÇ+?·?®?­?+%?Ëÿ±éݶ¥?wÿº{.nÇ+?·¥?{±ýÈ?³ø§¶?¡Ü¨}©?²Æ zÚ&j:+v?¨þø¯ù®w¥þ?à2?Þ?¨è­Ú&¢)ß¡«a¶Úÿÿûàz¿äz¹Þ?ú+?ù???Ý¢jÿ?wèþf