[BUGS] BUG #7811: strlen(NULL) cause psql crash
The following bug has been logged on the website:
Bug reference: 7811
Logged by: Meng Qingzhong
Email address: 1584171...@qq.com
PostgreSQL version: 9.2.2
Operating system: Windows 7
Description:
I find a bug this time really and I have fixed it. The bug is in psql, my
oprating system is Windows 7, and the IDE is Visual Studio 2010. The version
of postgreSQL is 9.2.2
I give you a description about how to trigger this bug first：
(1) start the server with the command postgres -D pgdata
(2) start the client with the command psql
(3) close the server
(4) execute a query from the client slect *from t; . At this
time, the client detected that it lost the connection with the server.
(5) execute the following command from the client \?, then the
client will crash.
I have found the reason which caused that.
(1) When the client execute slect *from t; , it execute the
function ResetCancelConn() at line 364 in src\bin\psql\common.c ,and the
function set pset.db to NULL.
(2) When the client execute \?, it execute the function fprintf
at line 254 in help.c. The value returned by PQdb(pset.db) is an argument of
fprintf, and at this time PQdb returned NULL.
(3) This NULL was finally passed to strlen at line 779 in
snprintf.c through several simple fuction calls, so psql crashed.
I hava fixed the bug in the following way which may be not the best:
(1) add a string named strofnull, and in the function dopr in
file src\port\snprintf.c
char *strofnull=(null);
(2) add an if statment before calling fmtstr at about line 720 in
file src\port\snprintf.c
if (strvalue==NULL)
{
strvalue=strofnull;
}
--
Sent via pgsql-bugs mailing list (pgsql-bugs@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-bugs
Re: [BUGS] BUG #7811: strlen(NULL) cause psql crash
On 15.01.2013 16:18, 1584171...@qq.com wrote:
I give you a description about how to trigger this bug first：
(1) start the server with the command postgres -D pgdata
(2) start the client with the command psql
(3) close the server
(4) execute a query from the client slect *from t; . At this
time, the client detected that it lost the connection with the server.
(5) execute the following command from the client \?, then the
client will crash.
I have found the reason which caused that.
(1) When the client execute slect *from t; , it execute the
function ResetCancelConn() at line 364 in src\bin\psql\common.c ,and the
function set pset.db to NULL.
(2) When the client execute \?, it execute the function fprintf
at line 254 in help.c. The value returned by PQdb(pset.db) is an argument of
fprintf, and at this time PQdb returned NULL.
(3) This NULL was finally passed to strlen at line 779 in
snprintf.c through several simple fuction calls, so psql crashed.
Thanks for the report and debugging!
I hava fixed the bug in the following way which may be not the best:
(1) add a string named strofnull, and in the function dopr in
file src\port\snprintf.c
char *strofnull=(null);
(2) add an if statment before calling fmtstr at about line 720 in
file src\port\snprintf.c
if (strvalue==NULL)
{
strvalue=strofnull;
}
That'd change the behavior of all sprintf calls, not sure we want to go
there. Might not be a bad idea to avoid crashes if there are more bugs
like this, but one really should not pass NULL to sprintf to begin with.
I committed a local fix to help.c to print none as the database name
when not connected.
- Heikki
--
Sent via pgsql-bugs mailing list (pgsql-bugs@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-bugs
Re: [BUGS] BUG #7811: strlen(NULL) cause psql crash
Heikki Linnakangas hlinnakan...@vmware.com writes: That'd change the behavior of all sprintf calls, not sure we want to go there. I don't think so; especially since the proposed fix doesn't help on systems where we are using the native printf. I committed a local fix to help.c to print none as the database name when not connected. I think that patch could use more thought. As is, it will print connect to new database (currently none) which to me is claiming that we are connected to a database whose name is none. The quotes should not be used in this situation, and in fact it would be better for translatability if the whole message text were specialized to this case. I'd like to see it reading more like connect to new database (currently no connection) regards, tom lane -- Sent via pgsql-bugs mailing list (pgsql-bugs@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-bugs
Re: [BUGS] BUG #7811: strlen(NULL) cause psql crash
On 15.01.2013 20:10, Tom Lane wrote: Heikki Linnakangashlinnakan...@vmware.com writes: I committed a local fix to help.c to print none as the database name when not connected. I think that patch could use more thought. As is, it will print connect to new database (currently none) which to me is claiming that we are connected to a database whose name is none. The quotes should not be used in this situation, and in fact it would be better for translatability if the whole message text were specialized to this case. I'd like to see it reading more like connect to new database (currently no connection) Hmm, that'd introduce a new quite visible string into back-branches that needs to be translated, which I think we try to avoid. I'll change that in master, but back-branches? - Heikki -- Sent via pgsql-bugs mailing list (pgsql-bugs@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-bugs
Re: [BUGS] BUG #7811: strlen(NULL) cause psql crash
Heikki Linnakangas hlinnakan...@vmware.com writes: On 15.01.2013 20:10, Tom Lane wrote: I think that patch could use more thought. As is, it will print connect to new database (currently none) which to me is claiming that we are connected to a database whose name is none. The quotes should not be used in this situation, and in fact it would be better for translatability if the whole message text were specialized to this case. I'd like to see it reading more like connect to new database (currently no connection) Hmm, that'd introduce a new quite visible string into back-branches that needs to be translated, which I think we try to avoid. But you already introduced none as a stand-alone (and probably almost untranslatable without context) string. That's better? regards, tom lane -- Sent via pgsql-bugs mailing list (pgsql-bugs@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-bugs
Re: [BUGS] BUG #7811: strlen(NULL) cause psql crash
On 15.01.2013 20:29, Tom Lane wrote: Heikki Linnakangashlinnakan...@vmware.com writes: On 15.01.2013 20:10, Tom Lane wrote: I think that patch could use more thought. As is, it will print connect to new database (currently none) which to me is claiming that we are connected to a database whose name is none. The quotes should not be used in this situation, and in fact it would be better for translatability if the whole message text were specialized to this case. I'd like to see it reading more like connect to new database (currently no connection) Hmm, that'd introduce a new quite visible string into back-branches that needs to be translated, which I think we try to avoid. But you already introduced none as a stand-alone (and probably almost untranslatable without context) string. That's better? I figured it would be. One little untranslated string in parens, versus the whole is untranslated. I'm happy to change it if you feel otherwise, though, I don't feel strongly about it myself. - Heikki -- Sent via pgsql-bugs mailing list (pgsql-bugs@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-bugs
Re: [BUGS] BUG #7811: strlen(NULL) cause psql crash
Heikki Linnakangas hlinnakan...@vmware.com writes: On 15.01.2013 20:29, Tom Lane wrote: But you already introduced none as a stand-alone (and probably almost untranslatable without context) string. That's better? I figured it would be. One little untranslated string in parens, versus the whole is untranslated. I'm happy to change it if you feel otherwise, though, I don't feel strongly about it myself. Well, I shouldn't be opining too strongly on translatability issues. Other people would have much-better-qualified opinions as to how well it'll read if none has to be translated by itself. But as to the behavior when the new message hasn't been translated yet: the only case where anyone would see the untranslated message is if they were in fact not connected, which we know is a seldom-exercised corner case (else this bug would've been noticed long ago). So it might not be worth arguing about. But ISTM that somebody whose English was weak might not grasp that none (untranslated) wasn't meant to be a name. regards, tom lane -- Sent via pgsql-bugs mailing list (pgsql-bugs@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-bugs
Re: [BUGS] BUG #7811: strlen(NULL) cause psql crash
On 15.01.2013 21:13, Tom Lane wrote: Heikki Linnakangashlinnakan...@vmware.com writes: On 15.01.2013 20:29, Tom Lane wrote: But you already introduced none as a stand-alone (and probably almost untranslatable without context) string. That's better? I figured it would be. One little untranslated string in parens, versus the whole is untranslated. I'm happy to change it if you feel otherwise, though, I don't feel strongly about it myself. Well, I shouldn't be opining too strongly on translatability issues. Other people would have much-better-qualified opinions as to how well it'll read if none has to be translated by itself. But as to the behavior when the new message hasn't been translated yet: the only case where anyone would see the untranslated message is if they were in fact not connected, which we know is a seldom-exercised corner case (else this bug would've been noticed long ago). So it might not be worth arguing about. But ISTM that somebody whose English was weak might not grasp that none (untranslated) wasn't meant to be a name. Hmm, I wonder if an empty string would be better? It'd look a bit odd, but at least it would not mislead you to think you're connected to a database called none. - Heikki -- Sent via pgsql-bugs mailing list (pgsql-bugs@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-bugs
Re: [BUGS] BUG #7811: strlen(NULL) cause psql crash
Heikki Linnakangas hlinnakan...@vmware.com writes: Hmm, I wonder if an empty string would be better? It'd look a bit odd, but at least it would not mislead you to think you're connected to a database called none. Works for me as a back-branch fix, especially since then we're adding zero new strings to the back branches. regards, tom lane -- Sent via pgsql-bugs mailing list (pgsql-bugs@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-bugs
