Skip to content
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

fails hot patching mysql_execute_command for debian mysql #132

Closed
lurdan opened this issue Jan 6, 2016 · 9 comments
Closed

fails hot patching mysql_execute_command for debian mysql #132

lurdan opened this issue Jan 6, 2016 · 9 comments

Comments

@lurdan
Copy link

lurdan commented Jan 6, 2016

hi,

Like #72, audit plugin fails hot patching with debian mysql package (jessie i686, 5.5.46-0+deb8u1)

With either distributed audit-plugin-mysql-5.5-1.0.9-545-linux-i386.zip or self-compiled binary from master branch, errors are same.

160106 12:47:00 [Note] Audit Plugin: mem func addr: 0xab163220 mem start addr: 0xab164000 page size: 4096
160106 12:47:00 [Note] Audit Plugin: hot patching function: 0xb6ddc8c0, trampolineFunction: 0xab164000 trampolinePage: 0xab164000
160106 12:47:00 [ERROR] Audit Plugin: unable to disassemble at address: 0x0xb6ddc8c4. Found relative addressing for instruction: [call 0xb6d69040]. Aborting.
160106 12:47:00 [ERROR] Audit Plugin: unable to hot patch mysql_execute_command (0xb6ddc8c0). res: -1.
160106 12:47:00 [ERROR] Plugin 'AUDIT' init function returned error.

I've also tried with extracted offsets from rebuilt binary, but no luck (same error).

Any clues?

@aharonrobbins
Copy link

Hi. Thanks for the report. We will be investigating this. It appears to be something specific to this distribution and version of MySQL.

@aharonrobbins
Copy link

This is a problem specific to Debian's build of MySQL. We will be investigating how to work around / fix the issue for a future release of the plugin. You might want to try using the distribution for Debian from mysql.org instead of using Debian's build.

Thanks again for the report.

@kubo
Copy link
Contributor

kubo commented Jun 28, 2016

I downloaded a debian mysql package for i386 and checked instructions around mysql_execute_command.
The function called at mysql_execute_command + 4 is:

  1660f0:   8b 1c 24                mov    (%esp),%ebx
  1660f3:   c3                      ret    

The instructions are same with __x86.get_pc_thunk.bx in http://ewontfix.com/18/.
It is called to get the program counter (PC) of mysql_execute_command when mysqld is compiled as a position independent executable.

IMO it will be fixed by this patch.
Well, I have not tested whether it works. Moreover I have not tested whether C compiler can compile it. Thus I just posted the patch instead of making a pull request.

@aharonrobbins
Copy link

Hi.

Much thanks for this! We will try to test it over the next few days and let you know.

Aharon

Aharon (Arnold) Robbins
Senior Software Engineer
Sensor - Database Security
McAfee. Part of Intel Security.
[cid:image003.png@01CF5AF2.16A4D380]

From: Kubo Takehiro [mailto:notifications@github.com]
Sent: Tuesday, June 28, 2016 12:01
To: mcafee/mysql-audit mysql-audit@noreply.github.com
Cc: Robbins, Aharon aharon.robbins@intel.com; Comment comment@noreply.github.com
Subject: Re: [mcafee/mysql-audit] fails hot patching mysql_execute_command for debian mysql (#132)

I downloaded a debian mysql package for i386 and checked instructions around mysql_execute_command.
The function called at mysql_execute_command + 4 is:

1660f0: 8b 1c 24 mov (%esp),%ebx

1660f3: c3 ret

The instructions are same with __x86.get_pc_thunk.bx in http://ewontfix.com/18/.
It is called to get the program counter (PC) of mysql_execute_command when mysqld is compiled as a position independent executable.

IMO it will be fixed by this patchhttps://gist.github.com/kubo/5a886f10d5a1879a12eed4ad9353c806.
Well, I have not tested whether it works. Moreover I have not tested whether C compiler can compile it. Thus I just posted the patch instead of making a pull request.


You are receiving this because you commented.
Reply to this email directly, view it on GitHubhttps://github.com//issues/132#issuecomment-228992541, or mute the threadhttps://github.com/notifications/unsubscribe/AKT-vUuZj5V0NMlILxqxS8ufLiIJjl8Zks5qQOLKgaJpZM4G_Vy1.

@lurdan
Copy link
Author

lurdan commented Jul 4, 2016

FYI:
I've tried @kubo's patch to compile, and got "jump to label" error because it skips the initializations of "*pCurInstr", just above second hunk.

It seems to be needed to reconstruct if-else-flow.

@aharonrobbins
Copy link

Thanks for the note.

We had to rearrange the source a little but got it to compile and are testing it now. We have not forgotten about it.

Thanks!

Aharon

Aharon (Arnold) Robbins
Senior Software Engineer
Sensor - Database Security
McAfee. Part of Intel Security.
[cid:image003.png@01CF5AF2.16A4D380]

From: lurdan [mailto:notifications@github.com]
Sent: Monday, July 04, 2016 07:24
To: mcafee/mysql-audit mysql-audit@noreply.github.com
Cc: Robbins, Aharon aharon.robbins@intel.com; Comment comment@noreply.github.com
Subject: Re: [mcafee/mysql-audit] fails hot patching mysql_execute_command for debian mysql (#132)

FYI:
I've tried @kubohttps://github.com/kubo's patch to compile, and got "jump to label" error because it skips the initializations of "*pCurInstr", just above second hunk.

It seems to be needed to reconstruct if-else-flow.


You are receiving this because you commented.
Reply to this email directly, view it on GitHubhttps://github.com//issues/132#issuecomment-230202215, or mute the threadhttps://github.com/notifications/unsubscribe/AKT-vfEofkzuUlEiAAWuNF2-qYVd0fGAks5qSIrmgaJpZM4G_Vy1.

@kubo
Copy link
Contributor

kubo commented Jul 5, 2016

The following code should be added to the patch.

                if (memcmp(callee, "\x8b\x0c\x24\xc3", 4) == 0) {
                    // If the current instruction is "call callee"
                    // and the callee is "movl (%esp), %ecx; ret",
                    // use "movl pc + 5, %ecx" instead.
                    BYTE *dest = (BYTE *)trampolineFunction + uCurrentSize;
                    *dest = 0xb9;
                    *(DWORD*)(dest + 1) = (DWORD)(pc + 5);
                    uCurrentSize += 5; // size of "movl pc + 5, %ecx"
                    InstrSize += 5;    // size of "call callee"
                    goto after_copy_instruction;
                 }

I'm writing my own hot-patching library(here) since last weekend.
When I tested it, I found that C compiler sometimes generates get_pc_thunk code which sets program counter to not only %ebx but also %ecx.
See https://github.com/kubo/duckhook/blob/57be9e1/src/duckhook_x86.c#L121-L138
It may be better to add similar code for other general registers: %eax, %edx and so on.

@aharonrobbins
Copy link

Hi. We have just pushed code that integrates @kubo's code. It works for me. Thanks!

@lurdan
Copy link
Author

lurdan commented Aug 1, 2016

I've confirmed that latest master can build and works on debian jessie i386. Thank you all!

@lurdan lurdan closed this as completed Aug 1, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants