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: <51A8D342.2010802@unix.sh>
Date:	Fri, 31 May 2013 10:43:46 -0600
From:	Alan Robertson <alanr@...x.sh>
To:	David Miller <davem@...emloft.net>
CC:	netdev@...r.kernel.org, davem@...emloft.net, kuznet@....inr.ac.ru,
	vyasevic@...hat.com
Subject: Re: [PATCH net] rtnetlink: ndo_dflt_fdb_del() never works

On 05/31/2013 02:46 AM, David Miller wrote:
> From: Alan Robertson <alanr@...x.sh>
> Date: Thu, 30 May 2013 16:01:55 -0600
>
>> ndo_dflt_fdb_del is checking for a condition which is opposite that
>> which ndo_dflt_fdb_add enforces.  ndo_dflt_fdb_add declares an error
>> if (ndm->ndm_state && !(ndm->ndm_state) & NUD_PERMANENT)) - that is, if the
>> entry is static.  This is consistent with the failure error message.
>>
>> On the other hand, ndo_dflt_del() declares an error
>> if (ndm_state & NUD_PERMANENT) - which is inconsistent with the add
>> precondition, and inconsistent with its failure message text.
>> As it is now, you can't delete any entry which add allows to be added -
>> so entry deletion always fails.
>>
>> Signed-off-by: Alan Robertson <alanr@...x.sh>
> What about the ->ndm_state part of the add() test?  Why not include
> that in the del() check?
I had three different thoughts about this:
  1) Replicated the add check in the delete
  2) Do what I did - make it where you can only delete those really
marked as static
  3) Eliminate all delete checks

The problem is -- I'm not sure why the ndm->ndm_state check is in there
-- I don't know how to make that condition occur.  It has the feel of a
check to be made before things are fully initialized - or maybe even
just leftover cruft.

You could make the argument that option (3) is the best -- if it's been
added successfully, you ought to be able to delete it.

If someone with more knowledge would comment that would be much appreciated!

I'm happy to submit any of these three versions of the patch.  Let me
know what's wanted.


-- 
    Alan Robertson <alanr@...x.sh> - @OSSAlanR

"Openness is the foundation and preservative of friendship...  Let me claim from you at all times your undisguised opinions." - William Wilberforce
--
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

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ