New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

cyrdump: segmentation fault #2080

Closed
elliefm opened this Issue Aug 8, 2017 · 3 comments

Comments

Projects
None yet
1 participant
@elliefm
Contributor

elliefm commented Aug 8, 2017

Reported by Stephan Lauffer on the mailing list, trivially reproduced. Here's a backtrace:

Program received signal SIGSEGV, Segmentation fault.
0x00007ffff755552c in strhash (string=0x0) at lib/strhash.c:50
50	      while (*string)
(gdb) bt
#0  0x00007ffff755552c in strhash (string=0x0) at lib/strhash.c:50
#1  0x00007ffff7550f25 in hash_lookup (key=0x0, table=0x55555576bc70) at lib/hash.c:156
#2  0x00007ffff7b6d663 in subquery_run_global (query=0x55555576bc20, mboxname=0x0)
    at imap/search_query.c:747
#3  0x00007ffff7b6da7a in search_query_run (query=0x55555576bc20) at imap/search_query.c:864
#4  0x00007ffff7b1e18a in index_getuidsequence (state=0x55555576b360, searchargs=0x7fffffffc900, 
    uid_list=0x7fffffffc8f0) at imap/index.c:1723
#5  0x00005555555558a9 in dump_me (data=0x7fffffffdad0, rock=0x7fffffffeab0) at imap/cyrdump.c:194
#6  0x00007ffff7b43236 in find_cb (rockp=0x7fffffffe9f0, 
    key=0x55555576ae10 "user.anneser.ellie.deltest.587EA34Es.db", keylen=9, 
    data=0x7ffff7fee179 "%(A %(anne lrswipkxtecdan) I 74dddd9c-9f16-4f4b-a06a-2af85ae76f28 P default V 1449552609 F 1 M 1449552609)", datalen=106) at imap/mboxlist.c:2442
#7  0x00007ffff7829193 in myforeach (db=0x55555576a760, prefix=0x7fffffffe110 "user.anne", 
    prefixlen=0, goodp=0x7ffff7b42d24 <find_p>, cb=0x7ffff7b42ff0 <find_cb>, rock=0x7fffffffe9f0, 
    tidptr=0x0) at lib/cyrusdb_twoskip.c:1580
#8  0x00007ffff781ad1c in cyrusdb_foreach (db=0x55555576a740, prefix=0x7fffffffe110 "user.anne", 
    prefixlen=0, p=0x7ffff7b42d24 <find_p>, cb=0x7ffff7b42ff0 <find_cb>, rock=0x7fffffffe9f0, 
    tid=0x0) at lib/cyrusdb.c:237
#9  0x00007ffff7b43f1e in mboxlist_find_category (rock=0x7fffffffe9f0, 
    prefix=0x7fffffffe110 "user.anne", len=0) at imap/mboxlist.c:2729
#10 0x00007ffff7b44834 in mboxlist_do_find (rock=0x7fffffffe9f0, patterns=0x55555576a570)
    at imap/mboxlist.c:2917
#11 0x00007ffff7b44ac9 in mboxlist_findallmulti (namespace=0x7ffff7dcb3c0 <ns>, 
    patterns=0x55555576a570, isadmin=1, userid=0x0, auth_state=0x0, proc=0x55555555566b <dump_me>, 
    rock=0x7fffffffeab0) at imap/mboxlist.c:2984
#12 0x0000555555555550 in main (argc=2, argv=0x7fffffffebc8) at imap/cyrdump.c:117

@elliefm elliefm added 3.0 3.1 labels Aug 8, 2017

@elliefm elliefm self-assigned this Aug 8, 2017

@elliefm

This comment has been minimized.

Show comment
Hide comment
@elliefm

elliefm Aug 8, 2017

Contributor

Okay, it's barfing out because it depends on state->mboxname having been set, which it's not.

state->mboxname is only set if index_open() was called with a struct index_init * second argument, but in this case it was called with a NULL second argument, so it's not set.

Possible fixes:

  1. make index_open() always initialise state->mboxname (seems safe?)
  2. or, setup a struct index_init and pass it to index_open()

doc/internal/index-api.html says:

The index_init interface sucks. So does passing lots of parameters. For now, this will do! Just pass NULL if you're only reading, or use the code already in imapd and you'll be fine.

Which makes me think NULL is correct in this place, and the first solution is the desirable one

Contributor

elliefm commented Aug 8, 2017

Okay, it's barfing out because it depends on state->mboxname having been set, which it's not.

state->mboxname is only set if index_open() was called with a struct index_init * second argument, but in this case it was called with a NULL second argument, so it's not set.

Possible fixes:

  1. make index_open() always initialise state->mboxname (seems safe?)
  2. or, setup a struct index_init and pass it to index_open()

doc/internal/index-api.html says:

The index_init interface sucks. So does passing lots of parameters. For now, this will do! Just pass NULL if you're only reading, or use the code already in imapd and you'll be fine.

Which makes me think NULL is correct in this place, and the first solution is the desirable one

@elliefm

This comment has been minimized.

Show comment
Hide comment
@elliefm

elliefm Aug 8, 2017

Contributor

Okay that partially fixes it, but there's other mess in here too. Does no-one use cyrdump these days?

Contributor

elliefm commented Aug 8, 2017

Okay that partially fixes it, but there's other mess in here too. Does no-one use cyrdump these days?

@elliefm

This comment has been minimized.

Show comment
Hide comment
@elliefm

elliefm Aug 8, 2017

Contributor

Fixed by 14b3ec5 and 7b7baa0 on master, and 7e13330 and 1d44995 on cyrus-imapd-3.0

Contributor

elliefm commented Aug 8, 2017

Fixed by 14b3ec5 and 7b7baa0 on master, and 7e13330 and 1d44995 on cyrus-imapd-3.0

@elliefm elliefm closed this Aug 8, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment