netdev - Re: [PATCH 1/1] net/x25: fix address parsing bug in x25_parse_address

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]

Date:	Wed, 19 Jun 2013 17:59:28 -0700 (PDT)
From:	David Miller <davem@...emloft.net>
To:	steve.moorby@...world.com
Cc:	sasha.levin@...cle.com, akpm@...ux-foundation.org,
	paulmck@...ux.vnet.ibm.com, jiri@...nulli.us,
	linux-x25@...r.kernel.org, netdev@...r.kernel.org,
	linux-kernel@...r.kernel.org
Subject: Re: [PATCH 1/1] net/x25: fix address parsing bug in
 x25_parse_address_block.

From: Stephen Moorby <steve.moorby@...world.com>
Date: Wed, 19 Jun 2013 21:32:36 +0100

> This problem was discovered when a linux box was incorrectly rejecting
> calls from some X.25 equipment.  The problem was diagnosed to an incorrect
> address length calculation in 'x25_parse_address_block', the calculation
> did not account for the address digits being BCD encoded.  The correct
> calculation is already performed on line 155.
> 
> Patched on linux-next 18-Jun-2013
> Tested on 2.6.32-45-generic
> 
> Signed-off-by: Stephen Moorby <steve.moorby@...world.com>

This change has two problems:

1) If there is existing code that does the calculation correctly, don't
   get creative and express the calculation differently than the existing
   code.

2) If there are two places doing the same thing, write a helper function
   that does it in one place.

The exact reason this bug exists is because of code duplication, you are
making it even worse by writing the same calculation two different ways.

Please fix this up and resubmit, thanks.

--
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