Comment on attachment 216983 [details] [review]
patch fixing issue.

Looks good.  I'd suggest spaces around the minus sign, and putting your comment in description part of the commit message as well.

Created attachment 217029 [details] [review]
updated patch

Updated patch showing the fix I committed.  I just added this line to ensure that the '\0' character is set.

+                        (*response_text)[PAM_MAX_RESP_SIZE - 1] = '\0';

I think this is reasonable, but let me know if you have any comments and I can rework.

well, one thing I just noticed is this bug in the caller:

got_response = gdm_session_worker_process_pam_message (worker,                                                                      
messages[i],                                                                    &response_text);
                if (!got_response) {
                        if (response_text != NULL) {
                                memset (response_text, '\0', strlen (response_text));
                                g_free (response_text);
                        }

So we're calling g_free on a variable we specifically decided to return non glib allocation memory with.  Except, we aren't, because response_text can never not be NULL on failure.

So I'm going to drop those lines of code.

Created attachment 217032 [details] [review]
worker: drop bogus memset/g_free

gdm_session_worker_process_pam_message returns C allocated memory
on success that gets freed by pam later.

On failure it returns NULL.  We had code in teh failure path that
checked if the result was not NULL and then tried to g_free.

Drop this wrong code.

Comment on attachment 217032 [details] [review]
worker: drop bogus memset/g_free

Attachment 217032 [details] pushed as e4a718f - worker: drop bogus memset/g_free