linux-kernel - Re: [PATCH v2] staging: tidspbridge: protect dmm

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]

Date:	Tue, 28 Dec 2010 08:33:30 +0200
From:	Ohad Ben-Cohen <ohad@...ery.com>
To:	Felipe Contreras <felipe.contreras@...ia.com>
Cc:	linux-kernel@...r.kernel.org, linux-omap@...r.kernel.org,
	greg@...ah.com, omar.ramirez@...com, fernando.lugo@...com,
	nm@...com, ameya.palande@...ia.com, h-kanigeri2@...com
Subject: Re: [PATCH v2] staging: tidspbridge: protect dmm_map properly

On Mon, Dec 27, 2010 at 3:55 PM, Felipe Contreras
<felipe.contreras@...ia.com> wrote:
> So, effectively, serializing the proc_begin_dma() and proc_end_dma()
> would not affect anyone negatively for the time being.

You can never really tell who is using the kernel (or will be using
this kernel version), how and under which workload.

> For the long-term goal I agree with that approach, however, first, I
> think my patch should be applied, since it's fixing a problem using an
> already existing and actively excersised mechanism. In fact, I think
> this should be pushed to 2.6.37 as:
>
>  1) prevents a kernel panic
>  2) the issue is reproducible and clearly identified
>  3) the patch is small and obvious

Both patches are (IMHO). But frankly I don't mind your patch will be
applied now as long as it doesn't stay. I can rebase my patch against
it after it is applied, and send separately.

> This approach looks cleaner, however, we need a flag in
> remove_mapping_information() in order to force the removal, otherwise
> there will be memory leaks. Imagine a process crashes, and
> remove_mapping_information() returns -EBUSY.

Can't happen; both proc_*_dma() operations decrease the reference
count before exiting, so it's not up to the application.

> Sure, but I see this as a broader effort to have finer locking, part of
> this should be to remove the already existing proc_lock.

Having bad locking is not an excuse for adding more.

Anyway, as I said, I don't really mind your patch will be applied as
long as it is a temporary workaround.

Thanks,
Ohad.
--
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/