New python PIDL checks cause 2221 new Coverity warnings

Volker Lendecke Volker.Lendecke at SerNet.DE
Wed Aug 26 05:14:02 UTC 2015


Hi, Andrew!

Sorry, I don't agree with this being unavoidable and by
design. We had our security bugs in PIDL output in the past,
so I think it would be a really bad decision to leave all
these warnings and undefined behaviour in the code, because
it makes the Coverity output nearly unusable.

Shouldn't the PIDL output be sensitive to 32-bit or 64-bit
architecture? We have #defines for the sizes of basic types.
Also, most of the IDL types we define are specific bit
values.

Volker

On Wed, Aug 26, 2015 at 10:24:37AM +1200, Andrew Bartlett wrote:
> On Tue, 2015-08-25 at 07:57 +0200, Volker Lendecke wrote:
> > Hi, Andrew!
> > 
> > The new PIDL checks for generated python code cause tons of new 
> > warnings
> > in Coverity.
> > 
> > One random example pasted after this message, please take a look at 
> > the
> > lines with CID. The web interface is much more usable, I'm pasting 
> > this
> > example here for easy access.
> > 
> > Is there anything we can do about this? Filtering out all generated
> > python code would be one possibility, but I would rather not do it.
> 
> Yes, we expected the compiler to optimise this out.  That was a
> deliberate design pattern to avoid trying to teach PIDL the size and
> maximum values of 'long' and all the IDL types (both classes of which
> change size depending on the architecture), and instead chose to rely
> on the compiler to handle that.
> 
> The idea was that consistent code across all the types and
> architectures (duplicated only for signed/unsigned in one area), and so
> relying on the compiler to do it's work would be smarter than encoding
> the compiler's knowledge in PIDL, and risking getting that wrong. 
> 
> We were aware of the left-shift of 64 bits, which is why that specific
> case is hard-coded at the start of the conditional expression, and rely
> on the undefined left-shift result never being used.
> 
> The biggest challenge is that a number of cases only really come up on
> 32 bit system, where a python long can't hold a uint32_t (on 64 bit it
> can), and so PyInt_FromLong can fail.
> 
> In short, when writing it we didn't consider Coverity giving warnings
> about this, we just expected the compiler to optimise it.  
> 
> Thanks,
> 
> Andrew Bartlett
> 
> -- 
> -- 
> Andrew Bartlett
> https://samba.org/~abartlet/
> Authentication Developer, Samba Team         https://samba.org
> Samba Development and Support, Catalyst IT   
> https://catalyst.net.nz/services/samba
> 
> 
> 
> 
> 

-- 
SerNet GmbH, Bahnhofsallee 1b, 37081 Göttingen
phone: +49-551-370000-0, fax: +49-551-370000-9
AG Göttingen, HRB 2816, GF: Dr. Johannes Loxen
http://www.sernet.de, mailto:kontakt at sernet.de



More information about the samba-technical mailing list