google / packetdrill

The official Google release of packetdrill
GNU General Public License v2.0
887 stars 220 forks source link

No so_instance cleanup can be done if PacketDrill test fails #64

Open AmPaschal opened 1 year ago

AmPaschal commented 1 year ago

Hi,

I am working with using PacketDrill to test some network stacks using a shared object file. I just noticed that when a test line in the PacketDrill script fails, the application exits without calling the free() function in the shared object file.

At the conclusion of each test, state_free (defined in run.c:122) is called and this frees the netdev and packet buffers and calls the free function in the so_instance where subsequent buffers in the shared object file can be freed.

    close_all_fds(state);

    netdev_free(state->netdev);
    packets_free(state->packets);
    code_free(state->code);

    if (state->wire_client)
        wire_client_free(state->wire_client);

    if (state->so_instance)
        so_instance_free(state->so_instance);

In run_system_call.c:3338, If an error occurs, the die function is called

die("%s:%d: runtime error in %s call: %s\n",
      state->config->script_path, event->line_number,
      syscall->name, error);
free(error);

And in the implementation of die in logging.c, no function is called to free the state

extern void __attribute__((noreturn)) die(char *format, ...)
{
    va_list ap;

    va_start(ap, format);
    vfprintf(stderr, format, ap);
    va_end(ap);

    run_cleanup_command();

    exit(EXIT_FAILURE);
}

As a result, when an error occurs during a test, the cleanup is not done completely.

Is this an expected behavior? If I want to call a function on my shared object file at the failure of each test, does the current PacketDrill implementation provide any support?

AmPaschal commented 1 year ago

If this is a valid bug, I'll be happy to implement a fix and send a pull request as well.

nealcardwell commented 1 year ago

Thanks for the report!

Given that the test failed and the process is about to exit, can you please outline your motivation for wanting to call a function on your shared object file at the failure of the test? Do you want to call the so_instance_free() function, or some new function for this case?

I'd just like to understand the details of the motivation and goal, to understand the priority of this issue.

best regards, neal

On Sat, Aug 13, 2022 at 4:16 AM AmPaschal @.***> wrote:

If this is a valid bug, I'll be happy to implement a fix and send a pull request as well.

— Reply to this email directly, view it on GitHub, or unsubscribe. You are receiving this because you are subscribed to this thread.Message ID: @.***>

AmPaschal commented 1 year ago

Hi Neal, thank you for getting back to me.

First, I am using PacketDrill to test a specific TCP stack. I use a shared object file to send syscalls and packets to the network stack being tested. After each test, I need to reset some variables in the network stack and clear some buffers as well. I currently do this in the free function which is defined in the .so file and called from the so_instance_free function. This enables me to run another PacketDrill script without restarting my application.

But whenever a script fails, PacketDrill exits without calling the so_instance_free function, and hence, my implemented logic to reset the state of my network stack doesn't run. Running the next PacketDrill script fails unless I manually restart my network stack. I want to subsequently use PacketDrill for fuzzing the network stack and this behavior may not work for me (I need to run multiple PacketDrill scripts without restarting the application being tested).

I look forward to hearing from you again. Please, let me know if there is any other way I can handle this with PacketDrill.

nealcardwell commented 1 year ago

Hi,

Just to make sure I understand the concern, can you please be more specific about the behavior you are hoping for in your use case, or propose a patch?

Do you just need die() to call state_free(), or do you need die() to not exit() as well?

thanks, neal

On Mon, Aug 15, 2022 at 12:32 AM AmPaschal @.***> wrote:

Hi Neal, thank you for getting back to me.

First, I am using PacketDrill to test a specific TCP stack. I use a shared object file to send syscalls and packets to the network stack being tested. After each test, I need to reset some variables in the network stack and clear some buffers as well. I currently do this in the free function which is defined in the .so file and called from the so_instance_free function. This enables me to run another PacketDrill script without restarting my application.

But whenever a script fails, PacketDrill exits without calling the so_instance_free function, and hence, my implemented logic to reset the state of my network stack doesn't run. Running the next PacketDrill script fails unless I manually restart my network stack. I want to subsequently use PacketDrill for fuzzing the network stack and this behavior may not work for me (I need to run multiple PacketDrill scripts without restarting the application being tested).

I look forward to hearing from you again. Please, let me know if there is any other way I can handle this with PacketDrill.

— Reply to this email directly, view it on GitHub https://github.com/google/packetdrill/issues/64#issuecomment-1214608620, or unsubscribe https://github.com/notifications/unsubscribe-auth/ACZHSHLDXZA6W2EL4GLVIITVZHB67ANCNFSM56NYKBAQ . You are receiving this because you commented.Message ID: @.***>

AmPaschal commented 1 year ago

Yes The behavior I want: so_instance_free() should always be called before PacketDrill exits, either after successful execution or because of a failure. My idea is to just call state_free() from die(), just after run_cleanup_command() but before exiting the application. I can propose a patch containing this for you to review.

AmPaschal commented 1 year ago

Modifying PacketDrill to create the behavior I needed was a bit more challenging than I thought. I can't call state_free when an error occurs because it would fail if there's any other blocking system call still in progress. Instead, I think I found a way to design the behavior I want without needing to modify PacketDrill. Thank you for your assistance. I will be closing this issue.