linux-kernel - Re: [PATCH] Samsung S3C24xx SD/MMC driver

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20071219175433.1ca7cd79@poseidon.drzeus.cx>
Date:	Wed, 19 Dec 2007 17:54:33 +0100
From:	Pierre Ossman <drzeus-mmc@...eus.cx>
To:	Harald Welte <laforge@...nmoko.org>
Cc:	linux-kernel@...r.kernel.org, tk@...ntech.de, ben-linux@...ff.org
Subject: Re: [PATCH] Samsung S3C24xx SD/MMC driver

On Wed, 19 Dec 2007 15:22:13 +0100
Harald Welte <laforge@...nmoko.org> wrote:

> Hi!
> 
> This is a MMC/SD driver for the Samsung S3C24xx SD/MMC controller, originally
> developed years ago by Thomas Kleffel <tk@...ntech.de>.
> 
> Due to time constraints, he had no time to further maintain the driver
> and follow the mainline Linux changes in the SD/MMC stack.
> 
> With his authorization, I have taken over the task of making it
> compliant to the current mainline SD/MMC API and take care of the
> mainline kernel merge.
> 
> So please advise on whatever changes you deem neccessary and I'll try to
> do my best to incorporate them for a hopefully not-too-distant mainline
> merge.
> 
> After a potential kernel inclusion, we would co-maintain the driver.
> 
> Acked-by: Thomas Kleffel <tk@...ntech.de>
> Signed-off-by: Harald Welte <laforge@...nmoko.org>
> 
> ---

Well, my biggest beef with this driver is the excessive debug code. I did a cleanup in an older version of the driver, but it shouldn't be to difficult to do the same for this one. I've included my patch for reference.

> Index: linux-2.6/drivers/mmc/host/s3cmci.c
> ===================================================================
> --- /dev/null
> +++ linux-2.6/drivers/mmc/host/s3cmci.c

> +#include <linux/mmc/mmc.h>

This is always a warning sign. It should never be needed in a host driver.


> +
> +static inline int get_data_buffer(struct s3cmci_host *host,
> +				  u32 *words, u32 **pointer)
> +{
> +	struct scatterlist *sg;
> +
> +	if (host->pio_active == XFER_NONE)
> +		return -EINVAL;
> +
> +	if ((!host->mrq) || (!host->mrq->data))
> +		return -EINVAL;
> +
> +	if (host->pio_sgptr >= host->mrq->data->sg_len) {
> +		dbg(host, dbg_debug, "no more buffers (%i/%i)\n",
> +		      host->pio_sgptr, host->mrq->data->sg_len);
> +		return -EBUSY;
> +	}
> +	sg = &host->mrq->data->sg[host->pio_sgptr];
> +
> +	*words = sg->length >> 2;
> +	*pointer = page_address(sg_page(sg)) + sg->offset;
> +

The length might not be a multiple of four. And it might also be completely unaligned. Make sure you can either handle such requests, or fail them with -EINVAL.

Rgds
-- 
     -- Pierre Ossman

  Linux kernel, MMC maintainer        http://www.kernel.org
  PulseAudio, core developer          http://pulseaudio.org
  rdesktop, core developer          http://www.rdesktop.org

View attachment "s3c-dbg.patch" of type "text/x-patch" (4539 bytes)