On Sun, Feb 15, 2015 at 08:53:08PM +0000, Miod Vallat wrote:
> This ought to fix this problem:
This looks reasonable but needs a tweak if you want to keep the existing
behavior. In the existing code, it has this section above the copyout():
rw_enter_write(&sc->sc_lock);
if (request == SDIOCEXECMMC)
error = sdmmc_mmc_command(sc, &cmd);
else
error = sdmmc_app_command(sc, &cmd);
rw_exit(&sc->sc_lock);
if (error && !cmd.c_error)
cmd.c_error = error;
and returns 0 even if error is non-zero. With this diff, it will either
return the result of those functions or the result of copyout depending
on whether ucmd->c_data is non-zero. Using the result of copyout is
intentional, but not the result from sdmmc_{mmc,app}_command().
Could you set error=0 after the above chunk or use a new variable?
Also, one of the new lines has space indents rather than a tab.
> Index: sdmmc.c
> ===================================================================
> RCS file: /cvs/src/sys/dev/sdmmc/sdmmc.c,v
> retrieving revision 1.36
> diff -u -p -r1.36 sdmmc.c
> --- sdmmc.c 1 Nov 2014 16:32:06 -0000 1.36
> +++ sdmmc.c 15 Feb 2015 20:52:08 -0000
> @@ -749,7 +749,7 @@ sdmmc_ioctl(struct device *self, u_long
> struct sdmmc_command *ucmd;
> struct sdmmc_command cmd;
> void *data;
> - int error;
> + int error = 0;
>
> switch (request) {
> #ifdef SDMMC_DEBUG
> @@ -784,8 +784,9 @@ sdmmc_ioctl(struct device *self, u_long
> M_WAITOK | M_CANFAIL);
> if (data == NULL)
> return ENOMEM;
> - if (copyin(ucmd->c_data, data, ucmd->c_datalen))
> - return EFAULT;
> + error = copyin(ucmd->c_data, data, ucmd->c_datalen);
> + if (error != 0)
> + goto exec_done;
>
> cmd.c_data = data;
> cmd.c_datalen = ucmd->c_datalen;
> @@ -804,10 +805,10 @@ sdmmc_ioctl(struct device *self, u_long
> ucmd->c_flags = cmd.c_flags;
> ucmd->c_error = cmd.c_error;
>
> - if (ucmd->c_data && copyout(data, ucmd->c_data,
> - ucmd->c_datalen))
> - return EFAULT;
> + if (ucmd->c_data)
> + error = copyout(data, ucmd->c_data, ucmd->c_datalen);
>
> +exec_done:
> if (ucmd->c_data)
> free(data, M_TEMP, 0);
> break;
> @@ -815,7 +816,7 @@ sdmmc_ioctl(struct device *self, u_long
> default:
> return ENOTTY;
> }
> - return 0;
> + return error;
> }
> #endif
>
>
