linux-kernel - Re: [PATCH] sh/maple: clean maple bus code

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] [day] [month] [year] [list]

Message-ID: <20080728235727.GE28055@linux-sh.org>
Date:	Tue, 29 Jul 2008 08:57:27 +0900
From:	Paul Mundt <lethal@...ux-sh.org>
To:	Adrian McMenamin <adrian@...golddream.dyndns.info>
Cc:	linux-sh <linux-sh@...r.kernel.org>,
	LKML <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH] sh/maple: clean maple bus code

On Tue, Jul 29, 2008 at 12:44:16AM +0100, Adrian McMenamin wrote:
>  static struct mapleq *maple_allocq(struct maple_device *mdev)
>  {
>  	struct mapleq *mq;
>  
>  	mq = kmalloc(sizeof(*mq), GFP_KERNEL);
>  	if (!mq)
> -		return NULL;
> +		goto failed_nomem;
>  
>  	mq->dev = mdev;
>  	mq->recvbufdcsp = kmem_cache_zalloc(maple_queue_cache, GFP_KERNEL);
>  	mq->recvbuf = (void *) P2SEGADDR(mq->recvbufdcsp);
> -	if (!mq->recvbuf) {
> -		kfree(mq);
> -		return NULL;
> -	}
> +	if (!mq->recvbuf)
> +		goto failed_p2;

This is really the wrong thing to check, you want to be checking
mq->recvbufdcsp here, as kmem_cache_zalloc() will return an error, while
P2SEGADDR() casts away. I expect that if you toss a NULL pointer or
something similar at it you will just get the P2 base back anyways, it
was never intended for error cases.

I expect you will also want to document your locking strategy in detail
at the top of the file. You do have some corner cases that are handled
that are not immediately obvious at first glance, it would be good to
spell it out explicitly.
--
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/