netdev - Re: [PATCH] [CAIF-RFC 5/8-v2] CAIF Protocol Stack

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: <4AD3263C.9010605@babic.homelinux.org>
Date:	Mon, 12 Oct 2009 14:51:08 +0200
From:	Stefano Babic <stefano.babic@...ic.homelinux.org>
To:	sjur.brandeland@...ricsson.com
CC:	netdev@...r.kernel.org, kim.xx.lilliestierna@...ricsson.com,
	christian.bejram@...ricsson.com, daniel.martensson@...ricsson.com
Subject: Re: [PATCH] [CAIF-RFC 5/8-v2] CAIF Protocol Stack

sjur.brandeland@...ricsson.com wrote:
> From: Sjur Braendeland <sjur.brandeland@...ricsson.com>

Hi Sjur,

> diff --git a/net/caif/caif_chr.c b/net/caif/caif_chr.c

> +#define caif_assert(assert) BUG_ON(!(assert))

Do we need special assert for each module (cfglu_assert,
caif_assert,...) ? They are all defined in the same way.
At this point I have already set a comment about using BUG_ON in a
previous patch.

I see a mixed policy in this patch, using sometimes _assert and
sometimes directly BUG_ON, too.

> diff --git a/net/caif/chnl_chr.c b/net/caif/chnl_chr.c

> +	/* Lock in order to try to stop someone from opening the device
> +	   too early. The misc device has its own lock. We cannot take our
> +	   lock until misc_register() is finished, because in open() the
> +	   locks are taken in this order (misc first and then dev).
> +	   So anyone managing to open the device between the misc_register
> +	   and the mutex_lock will get a "device not found" error. Don't
> +	   think it can be avoided.
> +	 */
> +	mutex_lock_interruptible(&dev->mutex);

The return value is not checked and it must be, as in all other cases.

> +	/* Find device from name */
> +	dev = find_device(-1, name, 0);
> +	if (!dev)
> +		return -EBADF;
> +
> +

> +	mutex_lock_interruptible(&dev->mutex);

The return value of mutex_lock_interruptible() must be checked.

Stefano

-- 
stefano <stefano.babic@...ic.homelinux.org>
GPG Key: 0x55814DDE
Fingerprint 4E85 2A66 4CBA 497A 2A7B D3BF 5973 F216 5581 4DDE
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html