Comment 8 for bug 1817027

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?