On 09/10/14 23:40, Samuel Thibault wrote:
> Hello,
>
> Our openvpn server got out of free inodes in /tmp, making it quite
> completely nonworking. This is due to some codepath in multi.c which
> does not remove its temporary file (when a plugin callback returns an
> error, or a client-connect script returns an error). Please see the
> attached patch which fixes this.
>
> Samuel
>
>
> patch
>
>
> diff --git a/src/openvpn/multi.c b/src/openvpn/multi.c
> index 5910154..d0ed147 100644
> --- a/src/openvpn/multi.c
> +++ b/src/openvpn/multi.c
> @@ -1699,6 +1705,9 @@ multi_connection_established (struct multi_context *m,
> struct multi_instance *mi
> {
> msg (M_WARN, "WARNING: client-connect plugin call failed");
> cc_succeeded = false;
> + if (!platform_unlink (dc_file))
> + msg (D_MULTI_ERRORS, "MULTI: problem deleting temporary file:
> %s",
> + dc_file);
> }
> else
> {
> @@ -1757,7 +1766,12 @@ multi_connection_established (struct multi_context *m,
> struct multi_instance *mi
> ++cc_succeeded_count;
> }
> else
> - cc_succeeded = false;
> + {
> + cc_succeeded = false;
> + if (!platform_unlink (dc_file))
> + msg (D_MULTI_ERRORS, "MULTI: problem deleting temporary file:
> %s",
> + dc_file);
> + }
> script_failed:
> argv_reset (&argv);
> }
> Good catch! But I'm not convinced about this approach, from a code quality perspective. Now we have the unlinking of these temp files two places. In multi_client_connect_post() if the plug-in(s) didn't fail, or in multi_connection_established() if the plug-in failed. I think it would be better to move the unlink() code from multi_client_connect_post() into multi_connection_established(), where these temp files are created. This makes the code clearer and easier to understand. And you can have a label and goto statement in the error section, similar to what's already done (look for script_depr_failed: and script_failed:). -- kind regards, David Sommerseth
signature.asc
Description: OpenPGP digital signature
