Is this a barebox bootm memory release Bug ?

Is this a barebox bootm memory release Bug ?

[Gabor Janak (g.janak@agilion.de)]

Hi,

I have one little question.
In [barebox.git] / common / bootm.c Line 201

196        if (data->verify) {
197                 ret = uimage_verify(data->os);
198                 if (ret) {
199                         printf("Checking data crc failed with %s\n",
200                                         strerror(-ret));
201                         uimage_close(data->os);
202                         return ret;
203                 }
204         }

The uimage is closed but data->os is not set to NULL.

If this function is called from  bootm_boot in
472                 ret = handler->bootm(data);

and
482         if (data->os)
483                 uimage_close(data->os);

will crash free something ....
Will end in a possible
unable to handle paging request at address 0xfe148f0e

Is this a correct analyze ?
If yes, it's enough to add an data->os=NULL after uimage_close ?

I would ask this here before submitting a patch.

In best regards
Gabor Janak



[Hannover Messe]<http://www.hannovermesse.de/aussteller/agilion/H465093?source=dl_ban>

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

Re: Is this a barebox bootm memory release Bug ?

[Sascha Hauer]

Hi Gabor,

On Wed, Mar 18, 2015 at 04:28:26PM +0000, Gabor Janak (g.janak@agilion.de) wrote:
> Hi,
> 
> I have one little question.
> In [barebox.git] / common / bootm.c Line 201
> 
> 196        if (data->verify) {
> 197                 ret = uimage_verify(data->os);
> 198                 if (ret) {
> 199                         printf("Checking data crc failed with %s\n",
> 200                                         strerror(-ret));
> 201                         uimage_close(data->os);
> 202                         return ret;
> 203                 }
> 204         }
> 
> The uimage is closed but data->os is not set to NULL.
> 
> If this function is called from  bootm_boot in
> 472                 ret = handler->bootm(data);
> 
> and
> 482         if (data->os)
> 483                 uimage_close(data->os);
> 
> will crash free something ....
> Will end in a possible
> unable to handle paging request at address 0xfe148f0e
> 
> Is this a correct analyze ?
> If yes, it's enough to add an data->os=NULL after uimage_close ?

Yes, this seems to be a bug. Alternatively we could just skip the
uimage_close in the bootm_open_os_uimage error paths.

Sascha


-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox