We see a crash in __strlen_sse2 but that seems "expected" to me as we are
passing `src` to it.
And at the level above src is a null pointer.
---
Lets go up the stack, here the call to push_ucs2_talloc:
(ctx=ctx@entry=0x0, dest=dest@entry=0x7ffe76d586a8, src=src@entry=0x0,
converted_size=converted_size@entry=0x7ffe76d586a0)
Note that:
- src = "where to copy from"
and
- ctx = "The talloc memory context"
are both null.
I don't know a lot about talloc, but ctx is intentionally null in this case
(seen in the call).
But copying from the empty string will surely not work.
---
This call in turn comes from E_md4hash higher up in the stack.
The empty string breaking us is the variable password which could be a hint.
---
E_md4hash gets the password from stack above - it passed it cleanly.
E_md4hash (passwd=0x0, p16=p16@entry=0x7ffe76d58a70 "")
---
So looking one up is `create_volume_objectid`
This tries to get the password with the following call:
lp_servicename(talloc_tos(), SNUM(conn))
We know it gets a NULL pointer and passes that on.
---
I see many places where the return of lp_servicename is not checked either (bad
practise)?
But on others e.g. source3/printing/notify.c line 424 it is:
422 »···const char *sharename = lp_servicename(talloc_tos(), snum);
423
424 »···if (sharename)
425 »···»···notify_printer_status_byname(ev, msg_ctx, sharename, status);
This function seems to be part of the samba3 protocol itself as I find it
defined:
source3/include/proto.h
But nowhere implemented.
Here we also find that calling this "password" might be a red herring.
It seems it just uses the same E_md4hash function, but nothing indicates that:
lp_servicename(talloc_tos(), SNUM(conn))
Would return a password.
---
Summary:
IMHO create_volume_objectid should not blindly pass things to E_md4hash.
It should check the pointer and return an error if things are wrong.
I checked the latest 4.7 branch but it is still the same.
I don't know enough about lp_servicename really make any decisions, I'd expect
something like:
unsigned char *create_volume_objectid(connection_struct *conn, unsigned char
objid[16])
{
- E_md4hash(lp_servicename(talloc_tos(), SNUM(conn)),objid);
- return objid;
+ char* pw = lp_servicename(talloc_tos();
+ if (pw) {
+ E_md4hash(lp_servicename(talloc_tos(), SNUM(conn)),objid);
+ return objid;
+ } else {
+ return NULL;
+ }
}
Unfortunately the functions calling create_volume_objectid also do not
check the return value either. And they used it there as "src" in a
memcpy so it would fail very similarly with a fault.
I think we would need to report that upstream (i.e. to people who
understand what "lp_servicename(talloc_tos(), SNUM(conn))" does in that
context.
@Andreas what do you think about all of the above?
@Thomas - with the data I found here do you think you could file an
upstream bug and report back the bug ID here for tracking the issue and
resolution?
--
You received this bug notification because you are a member of Ubuntu
Bugs, which is subscribed to Ubuntu.
https://bugs.launchpad.net/bugs/1817027
Title:
samba crashes when uploading files
To manage notifications about this bug go to:
https://bugs.launchpad.net/ubuntu/+source/samba/+bug/1817027/+subscriptions
--
ubuntu-bugs mailing list
ubuntu-bugs@lists.ubuntu.com
https://lists.ubuntu.com/mailman/listinfo/ubuntu-bugs
