From mboxrd@z Thu Jan 1 00:00:00 1970 Message-ID: <4E95A28B.3060505@domain.hid> Date: Wed, 12 Oct 2011 16:22:03 +0200 From: Gilles Chanteperdrix MIME-Version: 1.0 References: <1316360236.2169.40.camel@domain.hid> <8236197CA3DA864F9EF20609D9887263B4D0@domain.hid> <4E80F2A0.9030500@domain.hid> In-Reply-To: Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit Subject: Re: [Xenomai-help] PSOS skin: mismatch in function signatures cause buffer overflow List-Id: Help regarding installation and common use of Xenomai List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Thomas De Schampheleire Cc: Xenomai-help@domain.hid, dietmar.schindler@domain.hid On 10/12/2011 04:03 PM, Thomas De Schampheleire wrote: > Hi, > > On Mon, Sep 26, 2011 at 11:46 PM, Gilles Chanteperdrix > wrote: >> On 09/19/2011 10:45 AM, Thomas De Schampheleire wrote: >>> Hi, >>> >>> On Mon, Sep 19, 2011 at 9:42 AM, Ronny Meeus wrote: >>>> On Mon, Sep 19, 2011 at 9:25 AM, wrote: >>>>>> From: xenomai-help-bounces@domain.hid [mailto:xenomai-help-bounces@domain.hid] >>>>>> On Behalf Of Philippe Gerum >>>>>> Sent: Sunday, September 18, 2011 5:37 PM >>>>>> ... >>>>>> Actually, we used to follow strictly the pSOS convention for this until >>>>>> 2.4.x, at which point we moved to name strings precisely because >>>>>> non-null terminated char[4] arrays would break the registry, the way you >>>>>> described. This is one of the rare situations where mimicking a useless >>>>>> limitation of the original API may be challenged by usability concerns >>>>>> in the new environment, and usability won in that case. The problem >>>>>> mostly comes from the fact that char[4] is automatically convertible to >>>>>> const char *, so we have no warning/error leading us to check the >>>>>> potentially problematic call sites. >>>>>> >>>>>> The concern about moving back to char[4] arrays - null-terminated if >>>>>> shorter - is for people who currently assign strings longer than 4 >>>>>> characters to name their objects. What could be done, is providing a >>>>>> build switch to select the accepted input, like >>>>>> --disable-psos-long-names to turn on char[4] interpretation. >>>>> >>>>> While I would prefer the switch --enable-psos-long-names, this sounds okay to me, the more so as it is more than people who use the pSOS skin without obeying pSOS rules deserve. >>>>> -- >>> [..] >>>>> >>>> >>>> Another option would be to introduce a run-time switch. >>>> The default behavior could be that long names are supported and if the >>>> "enable_short_names()" function is called, all names will the cut at 4 >>>> characters. >>>> The advantage of this dynamic switch is that different applications >>>> using the same library can use the mode they prefer. >>> >>> I would also be in favor of a runtime setting, rather than a >>> compile-time one. One cannot assume that all PSOS applications on a >>> system follow the same rules, or that there cannot be a mix of >>> 'legacy' PSOS applications and 'xenomai-style' ones. According to me, >>> a library should support all these uses. >> >> Hi, >> >> here is a patch which truncates identifiers to four characters and >> terminates them by a '\0' character, in order to avoid the issues at >> the origin of this thread. The patch also adds a variable >> "psos_long_names", 0 by default, which may be set to a non-zero value >> to enable the old behaviour (assume the identifier strings may be >> longer than 4 characters and are already null terminated). >> >> Could someone with the issue test it? > > It seems this slipped through the cracks, my apologies. We already > implemented this ourselves similarly, we didn't actually test your > patch so far. > > Now that we're trying out xenomai-forge, I ported this patch and tested it. > There are a few problems with it (also relevant to cobalt xenomai) > > * a bug in __psos_maybe_short_name so that only 3 characters are > retained (see below) > * the call to __psos_maybe_short_name also needs to be done in the > _ident functions > * although we don't use it, I think the pt_create and pt_ident > functions also need to be adapted > > Moreover, I wonder why you have made psos_long_names an exported > global variable. Sharing variables between two different pieces of > software is not very clean. I think it would be nicer with a get/set > function. Thanks for the review. The exported global variable is the simplest possible and sufficient implementation of this feature. You risk having a hard time convincing us otherwise. -- Gilles.