netdev - Re: [PATCH 2/5] drivers/net/ethernet/dec/tulip: Use standard __set_bit

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:	Thu, 14 Jun 2012 23:28:55 +0900
From:	Takuya Yoshikawa <takuya.yoshikawa@...il.com>
To:	Akinobu Mita <akinobu.mita@...il.com>
Cc:	Grant Grundler <grantgrundler@...il.com>,
	Takuya Yoshikawa <yoshikawa.takuya@....ntt.co.jp>,
	akpm@...ux-foundation.org, bhutchings@...arflare.com,
	grundler@...isc-linux.org, arnd@...db.de, benh@...nel.crashing.org,
	avi@...hat.com, mtosatti@...hat.com,
	linux-net-drivers@...arflare.com, netdev@...r.kernel.org,
	linux-kernel@...r.kernel.org, linux-arch@...r.kernel.org,
	kvm@...r.kernel.org
Subject: Re: [PATCH 2/5] drivers/net/ethernet/dec/tulip: Use standard
 __set_bit_le() function

On Thu, 14 Jun 2012 18:36:42 +0900
Akinobu Mita <akinobu.mita@...il.com> wrote:

> >> 1) while I agree with Akinobu and thank him for pointing out a
> >> _potential_ alignment problem, this is a separate issue and your
> >> existing patch should go in anyway. There are probably other drivers
> >> with _potential_ alignment issues. Akinobu could get credit for
> >> finding them by submitting patches after reviewing calls to set_bit
> >> and set_bit_le() - similar to what you are doing now.
> >
> > I prefer approach 1.
> >
> > hash_table is local in build_setup_frame_hash(), so if further
> > improvement is also required, we can do that locally there later.
> 
> This potential alignment problem is introduced by this patch.  Because
> the original set_bit_le() in tulip driver can handle unaligned bitmap.
> This is why I recommended it should be fixed in this patch.

The original set_bit_le() was used only in build_setup_frame_hash().

If it's clear that the table is aligned locally in the function, I do
not think the __potential__ problem is introduced by this patch.

As you can see from my response to Arnd in v1 thread, I knew the
alignment requirement at that time and checked the definition of
hash_table before using __set_bit_le().

> But please just ignore me if I'm too much paranoid.  And I'll handle
> this issue if no one wants to do it.

I'm open to suggestions.

But now that the maintainer who can test the driver on real hardware
has suggested this patch should go in, I won't change the patch without
any real issue.

I would thank you if you improve this driver later on top of that.

Thanks,
	Takuya
--
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