Re: sdmmc: fix malloc error handling in sdmmc_mem_send_scr()

[Tobias Heider]

On Mon, Jan 10, 2022 at 02:39:58PM +0000, Visa Hankala wrote:
> On Mon, Jan 10, 2022 at 03:21:49PM +0100, Tobias Heider wrote:
> > On Mon, Jan 10, 2022 at 01:41:53PM +0000, Visa Hankala wrote:
> > > On Mon, Jan 10, 2022 at 01:12:10PM +0100, Tobias Heider wrote:
> > > > sdmmc_mem_send_scr() tries to malloc() with M_NOWAIT and returns 0 on
> > > > error,  which leads to sdmmc_mem_sd_init() passing uninitialized stack
> > > > memory to sdmmc_mem_decode_scr().
> > > > The diff below makes sdmmc_mem_send_scr() return ENOMEM if malloc fails.
> > > > 
> > > > ok?
> > > 
> > > OK visa@
> > > 
> > > Isn't there a similar problem with M_NOWAIT in sdmmc_mem_sd_switch()?
> > > 
> > 
> > Right, here's an updated diff that fixes both.
> 
> Looks better. However, could the error branches return ENOMEM directly
> instead of using goto out?

Makes sense. I also fixed the one in sdmmc_mem_send_cxd_data().

diff --git a/sys/dev/sdmmc/sdmmc_mem.c b/sys/dev/sdmmc/sdmmc_mem.c
index fae8d63912d..d46b1d612be 100644
--- a/sys/dev/sdmmc/sdmmc_mem.c
+++ b/sys/dev/sdmmc/sdmmc_mem.c
@@ -466,7 +466,7 @@ sdmmc_mem_send_scr(struct sdmmc_softc *sc, uint32_t *scr)
 
        ptr = malloc(datalen, M_DEVBUF, M_NOWAIT | M_ZERO);
        if (ptr == NULL)
-               goto out;
+               return ENOMEM;
 
        memset(&cmd, 0, sizeof(cmd));
        cmd.c_data = ptr;
@@ -528,10 +528,8 @@ sdmmc_mem_send_cxd_data(struct sdmmc_softc *sc, int 
opcode, void *data,
        int error = 0;
 
        ptr = malloc(datalen, M_DEVBUF, M_NOWAIT | M_ZERO);
-       if (ptr == NULL) {
-               error = ENOMEM;
-               goto out;
-       }
+       if (ptr == NULL)
+               return ENOMEM;
 
        memset(&cmd, 0, sizeof(cmd));
        cmd.c_data = ptr;
@@ -608,7 +606,7 @@ sdmmc_mem_sd_switch(struct sdmmc_function *sf, int mode, 
int group,
 
        ptr = malloc(statlen, M_DEVBUF, M_NOWAIT | M_ZERO);
        if (ptr == NULL)
-               goto out;
+               return ENOMEM;
 
        memset(&cmd, 0, sizeof(cmd));
        cmd.c_data = ptr;