Development discussion of ISC NMSG, a general purpose binary message encapsulation toolkit

Dave Plonka | 2 May 2012 17:53

Picon

nmsgtool --readpres, which message types?


Hi folks,

Are there any message types that nmsgtool --readpres works with?

For instance, for the types I've tried it, nmsgtool exits with an
error but doesn't report the "function not implemented" message that
is mentioned in nmsgtool(1) man page.

I want to be able to synthesize individual messages from text for test
cases while debugging a problem that I get a SIGSEGV when writing a
message with the perl API Net::Nmsg[::Output].

Does anyone know for sure if "string" type fields in ".proto"
files works for nmsgtool and/or Net::Nmsg::Output?  (If I
make it a "bytes" field it works, if I make it string, I
get SIGSEGV in nmsg_output_write, _nmsg_message_serialize,
protobuf_c_message_pack_to_buffer, on this assertion in protobuf-c.c:

size_t
protobuf_c_message_pack_to_buffer (const ProtobufCMessage *message,
                                   ProtobufCBuffer  *buffer)
{
     unsigned i;
     size_t rv = 0;
     ASSERT_IS_MESSAGE (message);
     ...

So it seems there is some memory corruption when handling the string
field type.

(Continue reading)

Robert Edmonds | 3 May 2012 00:38

Re: nmsgtool --readpres, which message types?

Dave Plonka wrote:
> I want to be able to synthesize individual messages from text for test
> cases while debugging a problem that I get a SIGSEGV when writing a
> message with the perl API Net::Nmsg[::Output].

the "--readpres" code is a big hack and has probably bitrotted by now.
(i'll probably remove it from a future release.)  i would recommend
using pynmsg if you need to synthesize short test messages.

--

-- 
Robert Edmonds
edmonds <at> isc.org

Dave Plonka | 3 May 2012 01:36

Picon

SIGSEGV when serializing string fields in nmsg (was "Re: nmsgtool --readpres, which message types?")

Hi Robert,

On Wed, May 02, 2012 at 06:38:35PM -0400, Robert Edmonds wrote:
> Dave Plonka wrote:
> > I want to be able to synthesize individual messages from text for test
> > cases while debugging a problem that I get a SIGSEGV when writing a
> > message with the perl API Net::Nmsg[::Output].
> 
> the "--readpres" code is a big hack and has probably bitrotted by now.
> (i'll probably remove it from a future release.)  i would recommend
> using pynmsg if you need to synthesize short test messages.

Thanks Robert... but that basically means you recommend learning python. :)
Matt Sisk's perl interface to Nmsg i working pretty well for me.

For a couple days, I've been trying to track down an incorrect write
to memory (via gdb of the perl interpreter) that I isolated to the
nmsg code I think I've finally got it.

I'm seeing this problem, and often SIGSEGV (but sometimes other heinous
fatal errors), when an nmsg protocol buffer message contains a string
field. For example, if you add an "optional string" field in a .proto
(and add it as an "nmsg_msgmod_ft_string" in msgmod), the protocol
buffer field quantifier_offset (in the generated ".pb-c.c" file)
will be 0; apparently this is unlike with other protocol buffer
field types, where the quantifier offset will be an offset to a
"has_MEMBER" function.  (My guess this is because you don't need a
"has_MEMBER" function since you can test if the point is not NULL
for string fields.)

(Continue reading)

Robert Edmonds | 3 May 2012 03:34

Re: SIGSEGV when serializing string fields in nmsg (was "Re: nmsgtool --readpres, which message types?")

Dave Plonka wrote:
> What do you think?

like i said, the code that does the conversion from text is pretty
sketchy, and it doesn't surprise me that there are bugs in it.  i would
recommend using the nmsg_message API to create message payloads instead.

--

-- 
Robert Edmonds
edmonds <at> isc.org

paul vixie | 3 May 2012 03:36

Re: SIGSEGV when serializing string fields in nmsg (was "Re: nmsgtool --readpres, which message types?")

On 5/3/2012 1:34 AM, Robert Edmonds wrote:
> Dave Plonka wrote:
>> What do you think?
> like i said, the code that does the conversion from text is pretty
> sketchy, and it doesn't surprise me that there are bugs in it.  i would
> recommend using the nmsg_message API to create message payloads instead.

would you accept patches from mr. plonka if he wanted to fix up the
existing utility?

some of us find the generic text-to-nmsg processor useful.

--

-- 
"But I'm not done complaining." --Dagon, 2012

Robert Edmonds | 3 May 2012 04:31

Re: SIGSEGV when serializing string fields in nmsg (was "Re: nmsgtool --readpres, which message types?")

paul vixie wrote:
> On 5/3/2012 1:34 AM, Robert Edmonds wrote:
> > Dave Plonka wrote:
> >> What do you think?
> > like i said, the code that does the conversion from text is pretty
> > sketchy, and it doesn't surprise me that there are bugs in it.  i would
> > recommend using the nmsg_message API to create message payloads instead.
> 
> would you accept patches from mr. plonka if he wanted to fix up the
> existing utility?

perhaps, but i suspect that section of the codebase needs to be
substantially rewritten (and not just "fixed up") to use the
nmsg_message API rather than fooling around directly with protobuf-c
structures.  but i would rather just remove that functionality and bump
the ABI/API.

> some of us find the generic text-to-nmsg processor useful.

it's not enough for it to be useful if it encourages bad design.  if a
payload fails to be converted, what do you do?  you can ignore it and
write nothing to the nmsg output stream, or you can exit with a non-zero
exit code.  both of those choices are bad.

--

-- 
Robert Edmonds
edmonds <at> isc.org

Dave Plonka | 3 May 2012 20:27

Picon

Re: SIGSEGV when serializing string fields in nmsg (was "Re: nmsgtool --readpres, which message types?")

Hi Robert, et al.

On Wed, May 02, 2012 at 09:34:05PM -0400, Robert Edmonds wrote:
> Dave Plonka wrote:
> > What do you think?
> 
> like i said, the code that does the conversion from text is pretty
> sketchy, and it doesn't surprise me that there are bugs in it.  i would
> recommend using the nmsg_message API to create message payloads instead.

Perhaps my main point was muddled in the details.
In priority order:

1) I believe libnmsg has a bug that if ever there is a "string" field
   in the .proto, it performs a bad assingment into the protobuf
   structure (I believe trying to manipulate the protobuf structure
   according to the parallel structure in for nmsg) that clobbers a
   pointer to the descriptor, causing aborts (via ASSERT_IS_MESSAGE)
   or other misbehaviors in the protobuf level underneath nmsg.

   I have a concise example that gets SIGSEGV when the address 0x1
   is dereference, but basically it can be reproduced by adding an
   optional string field to any existing message, and then set that
   field to a value and try to write it out as nmsg output.

   I was blocked on this bug while doing data analysis for a conference
   manuscript submission, so I dug into the code for a couple days.
   I imagine I can patch it in another day or two.

(Continue reading)

Robert Edmonds | 3 May 2012 21:31

Re: SIGSEGV when serializing string fields in nmsg (was "Re: nmsgtool --readpres, which message types?")

Dave Plonka wrote:
> 1) I believe libnmsg has a bug that if ever there is a "string" field
>    in the .proto, it performs a bad assingment into the protobuf
>    structure (I believe trying to manipulate the protobuf structure
>    according to the parallel structure in for nmsg) that clobbers a
>    pointer to the descriptor, causing aborts (via ASSERT_IS_MESSAGE)
>    or other misbehaviors in the protobuf level underneath nmsg.
> 
>    I have a concise example that gets SIGSEGV when the address 0x1
>    is dereference, but basically it can be reproduced by adding an
>    optional string field to any existing message, and then set that
>    field to a value and try to write it out as nmsg output.
> 
>    I was blocked on this bug while doing data analysis for a conference
>    manuscript submission, so I dug into the code for a couple days.
>    I imagine I can patch it in another day or two.
> 
>    Perhaps this bug was unnoticed simply because none of the ISC
>    nmsg types currently contain string fields. (Matt Sisk's tests for
>    Net::Nmsg are driven from the ISC nsmg types, so I don't beleieve
>    that part of the code was previously visited.)

ok, i think i see the problem.  yes, there are no message schemas in the
ISC message types that use the protobuf "string" type.

> 2) To aid in debugging, I wanted to be able to produce an nmsg (file)
>    that contained string fields from text input, so that I could
>    determine that the problem was within libnmsg itself (rather than
>    the pynmsg or Net::Nmsg perl bindings.)
>

(Continue reading)

Dave Plonka | 3 May 2012 23:23

Picon

Re: SIGSEGV when serializing string fields in nmsg (was "Re: nmsgtool --readpres, which message types?")


Hi Robert,

On Thu, May 03, 2012 at 03:31:27PM -0400, Robert Edmonds wrote:
> Dave Plonka wrote:
> > 1) I believe libnmsg has a bug that if ever there is a "string" field
> >    in the .proto, it performs a bad assingment into the protobuf
> >    structure (I believe trying to manipulate the protobuf structure
> >    according to the parallel structure in for nmsg) that clobbers a
> >    pointer to the descriptor, causing aborts (via ASSERT_IS_MESSAGE)
> >    or other misbehaviors in the protobuf level underneath nmsg.
<snip>
> >    Perhaps this bug was unnoticed simply because none of the ISC
> >    nmsg types currently contain string fields. (Matt Sisk's tests for
> >    Net::Nmsg are driven from the ISC nsmg types, so I don't beleieve
> >    that part of the code was previously visited.)
> 
> ok, i think i see the problem.  yes, there are no message schemas in the
> ISC message types that use the protobuf "string" type.
<snip>
> > It may be possible for me to workaround using a bytes field.  But that
> > is a kludge I would only choose if you're not interested in looking
> > into the string field problem that I'm bumping into or accepting
> > a patch.
> 
> ok, so when you develop a libnmsg message module that's backed by a
> protocol buffer schema there are basically two "schemas" that you need
> to write: the actual protobuf schema, which gets compiled by the
> protobuf-c compiler (protoc-c), and a "glue" layer written in C.

(Continue reading)

paul vixie | 3 May 2012 23:46

Re: SIGSEGV when serializing string fields in nmsg (was "Re: nmsgtool --readpres, which message types?")

On 5/3/2012 9:23 PM, Dave Plonka wrote:
> ...
>
> I've changed my "string" fields to "bytes" in my ".proto" file and
> it works now.
>
> I'm getting the feeling I should have come visited ISC to write this code. :)

you'd be welcome. though edmonds sits in atlanta not in isc hq.

--

-- 
"But I'm not done complaining." --Dagon, 2012

Robert Edmonds | 4 May 2012 00:14

Re: SIGSEGV when serializing string fields in nmsg (was "Re: nmsgtool --readpres, which message types?")

Dave Plonka wrote:
> On Thu, May 03, 2012 at 03:31:27PM -0400, Robert Edmonds wrote:
> > i think what you might be missing is that the nmsg message module field
> > types are not simple aliases for protobuf field types with similar
> > names.  the correct way to define a protobuf-backed
> > nmsg_msgmod_ft_string field is to use a protobuf "bytes" field, not a
> > protobuf "string" field.  it's not a kludge, that's how it's supposed to
> > work.  evidently bad things happen if the protobuf "string" type is used
> > instead.
> 
> Dress me up and call me Sally.
> http://en.wikipedia.org/wiki/Principle_of_least_astonishment

ok, i'm familiar with POLA :)

> Ah, now I see this is documented in "nmsg-0.6.17/nmsg/msgmod.h",
> where "string" and is the only one of the msgmod types that does not
> correspond to the protobuf type of the same name (when there is one
> of the same name):

hm, well, 4 out of 13 of the names for nmsg_msgmod_field_type values
have no protobuf equivalents (mlstring, ip, uint16, int16).  they're
built on top of protobuf fields, granted.

>         /** Protobuf byte array. */
>         nmsg_msgmod_ft_bytes,
> 
>         /**
>          * Protobuf byte array.
>          * String should not contain newlines.

(Continue reading)

Kyle Creyts | 3 May 2012 05:06

Re: SIGSEGV when serializing string fields in nmsg (was "Re: nmsgtool --readpres, which message types?")

That is what is done with the .ipdg_to_payload for handling pcap, right? Actually, does that have to be
rewritten, too?

Robert Edmonds <edmonds <at> isc.org> wrote:

paul vixie wrote:
> On 5/3/2012 1:34 AM, Robert Edmonds wrote:
> > Dave Plonka wrote:
> >> What do you think?
> > like i said, the code that does the conversion from text is pretty
> > sketchy, and it doesn't surprise me that there are bugs in it.  i would
> > recommend using the nmsg_message API to create message payloads instead.
> 
> would you accept patches from mr. plonka if he wanted to fix up the
> existing utility?

perhaps, but i suspect that section of the codebase needs to be
substantially rewritten (and not just "fixed up") to use the
nmsg_message API rather than fooling around directly with protobuf-c
structures.  but i would rather just remove that functionality and bump
the ABI/API.

> some of us find the generic text-to-nmsg processor useful.

it's not enough for it to be useful if it encourages bad design.  if a
payload fails to be converted, what do you do?  you can ignore it and
write nothing to the nmsg output stream, or you can exit with a non-zero
exit code.  both of those choices are bad.

--

--

(Continue reading)

Robert Edmonds | 3 May 2012 05:15

Re: SIGSEGV when serializing string fields in nmsg (was "Re: nmsgtool --readpres, which message types?")

Kyle Creyts wrote:
> That is what is done with the .ipdg_to_payload for handling pcap,
> right? Actually, does that have to be rewritten, too?

no, ipdg_to_payload is handled in the message module itself.

--

-- 
Robert Edmonds
edmonds <at> isc.org

Return

Return to gmane.network.nmsg.devel.

Project Web Page

Development discussion of ISC NMSG, a general purpose binary message encapsulation toolkit

Search Archive

Language

Change language

Options

Current view: Threads only / Showing only 20 lines / Not hiding cited text.
Change to All messages, whole messages, or hide cited text.

Post a message
NNTP Newsgroup
Classic Gmane web interface
XML

XML

RSS Feed
List Information

About Gmane