linux-kernel - Re: [Patch 1/1 v2] via-sdmmc: VIA MSP card reader driver support

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 for Android: free password hash cracker in your pocket

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

Message-Id: <200902101318.08517.arnd@arndb.de>
Date:	Tue, 10 Feb 2009 13:18:08 +0100
From:	Arnd Bergmann <arnd@...db.de>
To:	JosephChan@....com.tw
Cc:	linux-kernel@...r.kernel.org, ben-linux@...ff.org
Subject: Re: [Patch 1/1 v2] via-sdmmc: VIA MSP card reader driver support

On Tuesday 10 February 2009, JosephChan@....com.tw wrote:
> The following patches provides VIA MSP SD/MMC card reader driver support.
> And these patches are based on kernel 2.6.29-rc4.
> Also, thanks for Arnd's previous suggestions.

Looks mostly good, just two more comments:

> --- a/drivers/mmc/host/via-sdmmc.h	1970-01-01 08:00:00.000000000 +0800
> +++ b/drivers/mmc/host/via-sdmmc.h	2009-02-10 21:34:18.000000000 +0800

The header file is used in only one place, which means that you could
move everything from there into via-sdmmc.c. This would make reading the
code easier later on.

> +
> +	if (data->flags & MMC_DATA_WRITE) {
> +		for (i = 0; i < len; i++) {
> +			tmpbuf = kmap_atomic(sg_page(&sg[i]), KM_BIO_SRC_IRQ);
> +			tmpbuf += sg[i].offset;
> +			memcpy(host->ddmabuf + offset, tmpbuf, sg[i].length);
> +			offset += sg[i].length;
> +			kunmap_atomic(tmpbuf, KM_BIO_SRC_IRQ);
> +		}
> +	}

This loop is broken for sg chaining. You should use for_each_sg() or
sg_next() instead of sg[i] indexing.

> +	if (data->flags & MMC_DATA_READ) {
> +		struct scatterlist *sg = data->sg;
> +		unsigned char *sgbuf;
> +		int i;
> +
> +		for (i = 0; i < data->sg_len; i++) {
> +			sgbuf = kmap_atomic(sg_page(&sg[i]), KM_BIO_SRC_IRQ);
> +			memcpy(sgbuf + sg[i].offset, host->ddmabuf + offset,
> +			       sg[i].length);
> +			offset += sg[i].length;
> +			kunmap_atomic(sgbuf, KM_BIO_SRC_IRQ);
> +		}
> +	}
> +

same here.

	Arnd <><
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/