-
Notifications
You must be signed in to change notification settings - Fork 467
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
cupsd segfaults when SLP browsing is enabled #259
Comments
CUPS.org User: mike OK, thanks for reporting this - that might be a reason that things have not been working as expected in recent releases. The patch you've submitted is not quite correct (at least it is not how we want to implement the fix), however I will update this STR with the official patch once the fix is created. Thanks for the report! |
CUPS.org User: mike Please try the attached patch to confirm that it fixes your problem. |
CUPS.org User: mike OK, new patch against 1.1.19 that cleans things up... Please let me know how you all make out... |
CUPS.org User: paul.cunningham.sun Where can I find the new version of the patch? |
CUPS.org User: mike Look under the "trouble report files" section of the STR page and download str259v2.patch... |
CUPS.org User: paul.cunningham.sun Hi, I applied the str259v2.patch to dirsrc.c in CUPs 1.1.19 but it failed to build with a link problem ..... Linking cupsd... I could not find cups_strcpy() in the source code so temporarily changed it to strcpy(), it then built okay. But when I ran this patched cupsd against slp, a printer add (with lpadmin) still locked up as before. Any ideas? I have checked that the patch was applied successfully but will check again now. Paul |
CUPS.org User: paul.cunningham.sun Hi, I did a bit of debugging on solaris 9 for the failure that I get and it looks like my problem is with null pointers for '*src' on the following lines ....
in function SendSLPBrowse() in module schedule/dirsvc.c. I think if any of the following are null, p->make_model, p->location or src = p->info, then the *src check will cause the problem (ie. there were on specified on the lpadmin command line). I applied the ammended patch dirsvc.c.patch and then it seems to work okay against SLP on Solaris. paul |
CUPS.org User: paul.cunningham.sun I spoke to soon, it now fails somewhere else after a printer has been added okay and the cupsd has been up and running for a while ...... stopped in _poll at 0xdd98d2c9 I will investigate this further on Monday. Any comments ? paul |
CUPS.org User: mike This looks like a malloc heap corruption problem; if you are working from CVS, make sure you update everything on Monday or try the 1.1.20rc1 release to see if the problem continues - the fd set size code was not compatible with Sun's odd implementation... I'll run tests against Valgrind on Linux to see if there are any pending issues... |
CUPS.org User: paul.cunningham.sun I have tidied up my patch to make it more understandable and efficient, and added it as dirsvc.c.patch.v2 I have now also built cups-1.1.20rc1 but when configured for slp it still fails with a malloc memory problem ..... (/ws/on81-tools/SUNWspro/SC6.1/bin/dbx) where This occurs after a printer has been added and the cupsd has been running for a few minutes .... seram# /opt/sfw/cups/bin/lpstat -v This malloc problem does not always occur in the same place. |
CUPS.org User: paul.cunningham.sun I think I have found the malloc problem now. The 'strcpy' in
I have done a patch for this. I have also identified a memory leak, function AttrCallback() uses GetSlpAttrVal() to get p->make_model, p->location and tmp (3 times). This memory is malloced by GetSlpAttrVal() but it does not seem to be freed off. My patch also frees of the tmp (using ClearString(), but Looking at the code for UpdateSLPBrowse() it calls SLFindAttrs() with AttrCallback() as the call back function, but UpdateSLPBrowse() seems to process the 'p' items within itself. I think it's possible that AttrCallback() may not have set them up by the time UpdateSLPBrowse() tries to process them (but I may be wrong - as I don't know the SLP API - see I have posted v3 of the patch : dirsvc.c.patch.v3 |
CUPS.org User: paul.cunningham.sun I have change my patch slightly to correct my error in not termination a string copied with strncpy(). The cupsd daemon with slp configured now seems to be running okay. |
CUPS.org User: mike OK, I've added your changes plus a couple buffer checks, and all seems happy with Valgrind, etc. We'll be doing a 1.1.20rc2 release with the combined changes so that others can do testing... |
CUPS.org User: christophe.saout At least on the server side (well, the cups daemon that actually has the printer) it seems to be more or less stable. A cups daemon on an other machine immediately segfaults when it's configured to support slp browsing and finds the printer on the other server. It reports to have found the printer (in the log file) and then dies. Is this enough information to reproduce? Tell me if you need more. Another thing: One time the printer suddenly disapperead from the slp directory agent, but the cups daemon was still running. |
CUPS.org User: christophe.saout Forgot to mention: The crash occurst with 1.1.20rc2 plain and with dirsvc.patch.v4 applied (with a little hand applying) |
CUPS.org User: paul.cunningham.sun Hi, I think I may have a fix for this later problem, ie. remote cupsd crashes when trying to setup printers read via SLP on another print server. It looks like a problem with a NULL pointer. Change dirsvc.c function ProcessBrowseData() as follows ... change from ...
to ...
It may also be a good idea to put NULL checks on all the other string parameters passed into the ... void function, ie. on uri, location and make_model, so that they do not have a similar problem in the future. I will update my dirsvc.c.patch later for cups-1.1.19 |
CUPS.org User: paul.cunningham.sun I have now posted the updated patch for dirsvc.v: dirsvc.c.patch.v5, it only deals with 'info' though. |
CUPS.org User: chtephan-cups.saout Yes, this fixes the crash. Thanks. I found out that this is triggered because the code forgets to retrieve the printer description from the SLP attributes. Patch posted (line numbers might be a bit off though). |
CUPS.org User: chtephan-cups.saout One more thing: When using slp only browsing the server disables browsing after some time due to "Transport endpoint not connected". It appears that BrowseSocket has to be initialized to -1 in BrowseStart when CUPS browsing is not enabled (adding an else clause to the big if or something). Also it is explicitly set to 0 in StopBrowsing. This should be -1 I assume (because it seems that fd 0 can be on the InputSet for some other reason). |
CUPS.org User: mike OK, I think we are synced up in CVS now; the 1.1.20rc3 release will have all of the combined changes, so let me know if there is anything missing from there (and patches against 1.1.20rc3 or CVS, please...) |
CUPS.org User: chtephan-cups.saout What about the last five text posts? I don't see any of these fixes in rc3... (I've posted a patch for my last post, it's just a small change from = 0 to = -1 and an added else statement anyway)? |
CUPS.org User: paul.cunningham.sun I can not see the last couple of patches either for the update of 'info' state, unless they have been done in a different way. |
CUPS.org User: chtephan-cups.saout There is no CVS access to cups, am I right (or blind)? Well, I've just posted the patch I was talking about. CUPS has now been rock solid (on both server and clients) using SLP with all patches applied! |
CUPS.org User: mike Sorry, things should be sorted out tonight - the primary CVS server changed but the mirror server wasn't updated... |
CUPS.org User: chtephan-cups.saout Sorry for posting again... but could you please also consider the last patch I posted? http://www.cups.org/strfiles/259/dirsvc-fdset-zero-fix.patch The problem description this patch fixes: |
CUPS.org User: mike Thanks, I've applied the patch to CVS; it will be part of the next release candidate! |
"str259.patch": Index: dirsvc.cRCS file: /development/cvs/cups/scheduler/dirsvc.c,v
- */
- valbuf[ptr2 - ptr1] = '\0';
- */
@@ -1692,7 +1690,7 @@
@@ -1717,19 +1715,17 @@ p->type = CUPS_PRINTER_REMOTE;
return (SLP_TRUE); |
"str259-corrected.patch": Index: dirsvc.cRCS file: /home/anoncvs/cups/scheduler/dirsvc.c,v
- valbuf[0] = '\0';
- */
- valbuf[ptr2 - ptr1] = '\0';
- */
return (-1);
@@ -1717,19 +1713,17 @@ p->type = CUPS_PRINTER_REMOTE;
return (SLP_TRUE); |
"str259v2.patch": Index: dirsvc.cRCS file: /development/cvs/cups/scheduler/dirsvc.c,v
if (bufused < 0)
if ((ptr1 = strstr(attrlist, tag)) != NULL) if ((ptr2 = strchr(ptr1,')')) != NULL)
@@ -1687,7 +1688,7 @@
@@ -1712,19 +1713,17 @@ p->type = CUPS_PRINTER_REMOTE;
return (SLP_TRUE); |
"dirsvc.c.patch": *** cups-1.1.19/scheduler/dirsvc.c.orig Fri Sep 5 13:34:34 2003 *** 886,901 ****
! /* - setgroups(0, NULL);
--- 886,906 ----
! setgroups(1, &Group);
*** 1507,1513 **** ! for (src = p->make_model, dst = make_model; _src;) ! //for (src = p->make_model, dst = make_model; _src;) *** 1520,1526 **** ! for (src = p->location, dst = location; _src;) ! //for (src = p->location, dst = location; _src;) *** 1533,1539 **** ! for (src = p->info, dst = info; _src;) ! //for (src = p->info, dst = info; _src;) *** 1636,1649 **** ! valbuf[0] = '\0';
--- 1644,1656 ---- ! ClearString(valbuf);
*** 1651,1675 ****
! if (valbuflen > (ptr2 - ptr1)) ! strncpy(valbuf, ptr1, ptr2 - ptr1); ! /* ! for (ptr1 = valbuf; _ptr1; ptr1 ++) ! return (0); --- 1658,1679 ----
! /* ! _valbuf = malloc(ptr2 - ptr1 + 1); ! /* ! for (ptr1 = _valbuf; *ptr1; ptr1 ++) ! return (0); *** 1687,1693 **** --- 1691,1697 ---- *** 1712,1730 ****
! if (GetSlpAttrVal(attrlist, "(printer-location=", p->location, ! if (GetSlpAttrVal(attrlist, "(color-supported=", tmp, sizeof(tmp))) ! if (GetSlpAttrVal(attrlist, "(finishings-supported=", tmp, sizeof(tmp)))
! if (GetSlpAttrVal(attrlist, "(printer-location=", &(p->location))) ! if (GetSlpAttrVal(attrlist, "(color-supported=", &tmp)) ! if (GetSlpAttrVal(attrlist, "(finishings-supported=", &tmp)) *** 1733,1743 **** ! if (GetSlpAttrVal(attrlist, "(sides-supported=", tmp, sizeof(tmp)))
} --- 1735,1747 ---- ! if (GetSlpAttrVal(attrlist, "(sides-supported=", &tmp))
|
"slp-get-printer-info.patch": --- cups-1.1.20rc2.orig/scheduler/dirsvc.c 2003-10-04 15:46:34.949573880 +0200 if (GetSlpAttrVal(attrlist, "(printer-location=", &(p->location)))
|
"dirsvc-fdset-zero-fix.patch": diff -Nur cups-1.1.19.orig/scheduler/dirsvc.c cups-1.1.19/scheduler/dirsvc.c
}
#ifdef HAVE_LIBSLP
|
Version: 1.1.19
CUPS.org User: seitz.ergon
On my Debian System cupsd segfaults when browsing using SLP is
switched on.
I identified a write to a NULL pointer as the problem.
A patch that seems to solve the problem follows:
diff -Naur cupsys-1.1.19-orig/scheduler/dirsvc.c cupsys-1.1.19-patched/scheduler/dirsvc.c
--- cupsys-1.1.19-orig/scheduler/dirsvc.c Mon May 12 22:51:53 2003
+++ cupsys-1.1.19-patched/scheduler/dirsvc.c Thu Aug 28 15:24:27 2003
@@ -1636,8 +1636,7 @@
int /* O - 0 on success /
GetSlpAttrVal(const char *attrlist, / I - Attribute list string /
const char *tag, / I - Name of attribute */
{
char ptr1, / Pointer into string /
*ptr2; / ... */
@@ -1651,7 +1650,7 @@
if ((ptr2 = strchr(ptr1,')')) != NULL)
{
{
/*
* Copy the value...
@@ -1659,7 +1658,6 @@
@@ -1708,23 +1706,20 @@
*/
memset(p, 0, sizeof(printer_t));
p->type = CUPS_PRINTER_REMOTE;
return (SLP_FALSE);
return (SLP_FALSE);
return (SLP_FALSE);
if (strcasecmp(tmp, "true") == 0)
p->type |= CUPS_PRINTER_COLOR;
return (SLP_FALSE);
if (strstr(tmp, "staple"))
p->type |= CUPS_PRINTER_STAPLE;
@@ -1733,7 +1728,7 @@
if (strstr(tmp, "punch"))
p->type |= CUPS_PRINTER_PUNCH;
return (SLP_FALSE);
if (strstr(tmp,"two-sided"))
p->type |= CUPS_PRINTER_DUPLEX;
@@ -1820,6 +1815,10 @@
resource[HTTP_MAX_URI]; /* Resource portion of URI /
int port; / Port portion of URI */
LogMessage(L_DEBUG, "UpdateSLPBrowse() Start...");
@@ -1893,6 +1892,8 @@
}
LogMessage(L_DEBUG, "UpdateSLPBrowse() End...");
}
The text was updated successfully, but these errors were encountered: