| Message ID | 20161221001731.19271-1-blp@ovn.org |
|---|---|
| State | Accepted |
| Headers | show |
----- Original Message ----- > From: "Ben Pfaff" <blp@ovn.org> > To: dev@openvswitch.org > Cc: "Ben Pfaff" <blp@ovn.org>, "Lance Richardson" <lrichard@redhat.com> > Sent: Tuesday, December 20, 2016 7:17:31 PM > Subject: [PATCH] ovsdb-idlc: Initialize nonnull string columns for inserted rows. > > When a schema column has type "exactly one string", the corresponding > struct member has type "char *" and the documented and expected behavior > is that the string should always be nonnull. (The code generator even > adds a comment /* Always nonnull. */ in the struct definition.) In the > case where a value is not available, the string is supposed to be > initialized to "" instead of to NULL. > > However, the IDL code for inserting a new row did not properly initialize > the column to "", instead leaving it NULL. This could cause null pointer > dereferences in corner cases. > > This commit fixes the problem. > > Reported-by: Lance Richardson <lrichard@redhat.com> > Reported-at: > https://mail.openvswitch.org/pipermail/ovs-dev/2016-December/326500.html > Signed-off-by: Ben Pfaff <blp@ovn.org> > --- > ovsdb/ovsdb-idlc.in | 5 +++++ > 1 file changed, 5 insertions(+) > > diff --git a/ovsdb/ovsdb-idlc.in b/ovsdb/ovsdb-idlc.in > index f1c7a35..721ab50 100755 > --- a/ovsdb/ovsdb-idlc.in > +++ b/ovsdb/ovsdb-idlc.in > @@ -452,6 +452,11 @@ void > for columnName, column in sorted_columns(table): > if column.type.is_smap(): > print " smap_init(&row->%s);" % columnName > + elif (column.type.n_min == 1 and > + column.type.n_max == 1 and > + column.type.key.type == ovs.db.types.StringType and > + not column.type.value): > + print " row->%s = \"\";" % columnName > print "}" > > # First, next functions. > -- > 2.10.2 > > Works for me, thanks! BTW, this was originally encountered for a CMS application using the Python IDL interface to execute transactions on the northbound DB. Tested-by: Lance Richardson <lrichard@redhat.com>
On Tue, Dec 20, 2016 at 07:32:58PM -0500, Lance Richardson wrote: > > > ----- Original Message ----- > > From: "Ben Pfaff" <blp@ovn.org> > > To: dev@openvswitch.org > > Cc: "Ben Pfaff" <blp@ovn.org>, "Lance Richardson" <lrichard@redhat.com> > > Sent: Tuesday, December 20, 2016 7:17:31 PM > > Subject: [PATCH] ovsdb-idlc: Initialize nonnull string columns for inserted rows. > > > > When a schema column has type "exactly one string", the corresponding > > struct member has type "char *" and the documented and expected behavior > > is that the string should always be nonnull. (The code generator even > > adds a comment /* Always nonnull. */ in the struct definition.) In the > > case where a value is not available, the string is supposed to be > > initialized to "" instead of to NULL. > > > > However, the IDL code for inserting a new row did not properly initialize > > the column to "", instead leaving it NULL. This could cause null pointer > > dereferences in corner cases. > > > > This commit fixes the problem. > > > > Reported-by: Lance Richardson <lrichard@redhat.com> > > Reported-at: > > https://mail.openvswitch.org/pipermail/ovs-dev/2016-December/326500.html > > Signed-off-by: Ben Pfaff <blp@ovn.org> > > --- > > ovsdb/ovsdb-idlc.in | 5 +++++ > > 1 file changed, 5 insertions(+) > > > > diff --git a/ovsdb/ovsdb-idlc.in b/ovsdb/ovsdb-idlc.in > > index f1c7a35..721ab50 100755 > > --- a/ovsdb/ovsdb-idlc.in > > +++ b/ovsdb/ovsdb-idlc.in > > @@ -452,6 +452,11 @@ void > > for columnName, column in sorted_columns(table): > > if column.type.is_smap(): > > print " smap_init(&row->%s);" % columnName > > + elif (column.type.n_min == 1 and > > + column.type.n_max == 1 and > > + column.type.key.type == ovs.db.types.StringType and > > + not column.type.value): > > + print " row->%s = \"\";" % columnName > > print "}" > > > > # First, next functions. > > -- > > 2.10.2 > > > > > > Works for me, thanks! BTW, this was originally encountered for a CMS application > using the Python IDL interface to execute transactions on the northbound DB. > > Tested-by: Lance Richardson <lrichard@redhat.com> Thanks, I applied this to master and branch-2.6. This is a generic IDL bug and so in theory it could crop up with older branches as well, in non-OVN contexts, but I don't expect that and so I didn't bother to backport it farther.
diff --git a/ovsdb/ovsdb-idlc.in b/ovsdb/ovsdb-idlc.in index f1c7a35..721ab50 100755 --- a/ovsdb/ovsdb-idlc.in +++ b/ovsdb/ovsdb-idlc.in @@ -452,6 +452,11 @@ void for columnName, column in sorted_columns(table): if column.type.is_smap(): print " smap_init(&row->%s);" % columnName + elif (column.type.n_min == 1 and + column.type.n_max == 1 and + column.type.key.type == ovs.db.types.StringType and + not column.type.value): + print " row->%s = \"\";" % columnName print "}" # First, next functions.
When a schema column has type "exactly one string", the corresponding struct member has type "char *" and the documented and expected behavior is that the string should always be nonnull. (The code generator even adds a comment /* Always nonnull. */ in the struct definition.) In the case where a value is not available, the string is supposed to be initialized to "" instead of to NULL. However, the IDL code for inserting a new row did not properly initialize the column to "", instead leaving it NULL. This could cause null pointer dereferences in corner cases. This commit fixes the problem. Reported-by: Lance Richardson <lrichard@redhat.com> Reported-at: https://mail.openvswitch.org/pipermail/ovs-dev/2016-December/326500.html Signed-off-by: Ben Pfaff <blp@ovn.org> --- ovsdb/ovsdb-idlc.in | 5 +++++ 1 file changed, 5 insertions(+)