dovecot - Mar 2011 - Regression in settings-parser.c (dovecot 2.0.11)

Hi,

one Fedora user sees regression in dovecot 2.0.11 (compared to prev. 2.0.9).
There was change recently:
http://hg.dovecot.org/dovecot-2.0/diff/bc77e80947c0/src/lib-settings/settings-parser.c
which adds call to settings_find_key_nth(...., &parent_def,...). This
function can set parent_def to NULL, but
there is no check for this situation in the code. So on next line if
(parent_def->type !=... dovecot crashes.
We've tested simple fix: if (parent_def==NULL) return FALSE; and everything
was working again.

Michal

Backtrace of the crash:
Program received signal SIGSEGV, Segmentation fault.
settings_find_key_nth (ctx=0x99e4050, key=0x99c4330 "plugin//etc", 
    n=0xbff9bdac, def_r=0xbff9bda8, link_r=0xbff9bda4) at settings-parser.c:696
696   if (parent_def->type != SET_STRLIST)
(gdb) p parent_def
$1 = (const struct setting_define *) 0x0
(gdb) bt
#0  settings_find_key_nth (ctx=0x99e4050, key=0x99c4330 "plugin//etc",
    n=0xbff9bdac, def_r=0xbff9bda8, link_r=0xbff9bda4) at settings-parser.c:696
#1  0x004f9ce5 in settings_find_key_nth (ctx=0x99e4050, 
    key=0x99c4318 "plugin//etc/dovecot", n=0xbff9be1c,
def_r=0xbff9be18,
    link_r=0xbff9be14) at settings-parser.c:693
#2  0x004f9ce5 in settings_find_key_nth (ctx=0x99e4050, 
    key=0x99c42f8 "plugin//etc/dovecot/users", n=0xbff9beb4,
def_r=0xbff9bebc,
    link_r=0xbff9beb8) at settings-parser.c:693
#3  0x004f94fd in settings_parse_keyvalue (ctx=0x99e4050, 
    key=0x99c42f8 "plugin//etc/dovecot/users", value=0x99c42f2
"yes")
    at settings-parser.c:760
#4  0x004fa270 in settings_parse_line (ctx=0x99e4050, 
    line=0x99c42d8 "plugin//etc/dovecot/users=yes") at
settings-parser.c:862
#5  0x00dededa in set_line (ctx=0x99cd468, input=0xbff9c100, 
    user_r=0xbff9c04c, error_r=0xbff9c0fc) at mail-storage-service.c:134
#6  user_reply_handle (ctx=0x99cd468, input=0xbff9c100, user_r=0xbff9c04c, 
    error_r=0xbff9c0fc) at mail-storage-service.c:227
#7  mail_storage_service_lookup (ctx=0x99cd468, input=0xbff9c100, 
    user_r=0xbff9c04c, error_r=0xbff9c0fc) at mail-storage-service.c:843
#8  0x00deeaae in mail_storage_service_lookup_next (ctx=0x99cd468, 
    input=0xbff9c100, user_r=0xbff9c0ac, mail_user_r=0xbff9c0a8, 
    error_r=0xbff9c0fc) at mail-storage-service.c:968
#9  0x0805f2e7 in client_create_from_input (input=<value optimized out>, 
---Type <return> to continue, or q <return> to quit---
    login_client=0x99ce810, fd_in=11, fd_out=11, input_buf=0xbff9c0e0, 
    error_r=0xbff9c0fc) at main.c:202
#10 0x0805f5dd in login_client_connected (client=0x99ce810, 
    username=0x99c409b "sjoerd", extra_fields=0x99c4110) at main.c:267
#11 0x0051606f in master_login_auth_finish (client=0x99ce810, 
    auth_args=<value optimized out>) at master-login.c:206
#12 0x005163c2 in master_login_auth_callback (auth_args=0x99c410c, 
    errormsg=0x0, context=0x99ce810) at master-login.c:374
#13 0x00516d3e in master_login_auth_input_user (auth=0x99cdcd8)
    at master-login-auth.c:239
#14 master_login_auth_input (auth=0x99cdcd8) at master-login-auth.c:359
#15 0x0052cc72 in io_loop_call_io (io=0x99cea10) at ioloop.c:384
#16 0x0052ded3 in io_loop_handler_run (ioloop=0x99cc390) at ioloop-epoll.c:213
#17 0x0052cbf0 in io_loop_run (ioloop=0x99cc390) at ioloop.c:405
#18 0x005181fb in master_service_run (service=0x99cc2e0, 
    callback=0x805f100 <client_connected>) at master-service.c:478
#19 0x0805fb0d in main (argc=1, argv=0xbff9c4c4) at main.c:375

On Fri, 2011-03-25 at 09:57 +0100, Michal Hlavinka
wrote:
> Hi,
> 
> one Fedora user sees regression in dovecot 2.0.11 (compared to prev.
2.0.9). There was change recently:
>
http://hg.dovecot.org/dovecot-2.0/diff/bc77e80947c0/src/lib-settings/settings-parser.c
> which adds call to settings_find_key_nth(...., &parent_def,...). This
function can set parent_def to NULL, but
> there is no check for this situation in the code. So on next line if
(parent_def->type !=... dovecot crashes.
> We've tested simple fix: if (parent_def==NULL) return FALSE; and
everything was working again.
Well, yes, there is a bug. But how is this happening?
> #3  0x004f94fd in settings_parse_keyvalue (ctx=0x99e4050, 
>     key=0x99c42f8 "plugin//etc/dovecot/users", value=0x99c42f2
"yes")
Where is this "plugin//etc/dovecot/users=yes" coming from? Is it from
a
userdb lookup that returns a broken setting? If Dovecot is somehow
internally setting this, there's a bug somewhere.

On Fri, 2011-03-25 at 09:57 +0100, Michal Hlavinka
wrote:
> Hi,
> 
> one Fedora user sees regression in dovecot 2.0.11 (compared to prev.
2.0.9). There was change recently:
>
http://hg.dovecot.org/dovecot-2.0/diff/bc77e80947c0/src/lib-settings/settings-parser.c
> which adds call to settings_find_key_nth(...., &parent_def,...). This
function can set parent_def to NULL, but
> there is no check for this situation in the code. So on next line if
(parent_def->type !=... dovecot crashes.
> We've tested simple fix: if (parent_def==NULL) return FALSE; and
everything was working again.
I fixed it with: http://hg.dovecot.org/dovecot-2.0/rev/746b4e9e7015

I'm not really sure what the handling should be when parent_def=NULL. I
think it might make a difference in future..