Control: found -1 2.3.4-1
Control: notfound -1 2.3.4
Control: tags -1 + moreinfo
Control: severity -1 important

Thanks for filing this bug report.

There is definitely a TOCTOU race condition here; the code immediately
before this checks for the file's presence by trying to open it read only,
and then if this fails, opens the file for writing - using flags that cause
it to *truncate* the file if it already exists.  This means that if the file
is created between the read check and the write attempt, it will be
overwritten with an empty file, which is wrong.

uo_fopen() takes three modes (plus one additional alias to one of the
modes); in addition to the 'r' and 'w' modes seen in this code, there is an
'a' mode which actually does what we want for this case: creates the file if
it does not exist, but does not truncate any existing file.  It is
straightforward to patch the code for this.

What is unclear is why in your case, "high load" causes there to be a race. 
The only time there should be a race is if one process or thread is checking
for the presence of the file, while another process or thread is first
deleting and then recreating the file.  From what I see in the code, it
appears that unixodbc only ever writes to the file under its final name -
which is a separate bug, because it means changes to odbc.ini are not
handled atomically on write, but that bug also means that I don't see any
way that this race condition would happen with unixodbc alone.  Do you know
what is causing your odbc.ini to be constantly rewritten?

I'm lowering the severity of this bug report because it appears to me that
the impact of this is nonexistent without a separate bug in an application.


On Wed, Dec 27, 2017 at 03:17:59PM +0100, Jan Jockusch wrote:
> Package: libodbc1
> Version: 2.3.4
> Severity: critical
> Tags: patch upstream
> Justification: causes serious data loss
> 
> Dear Maintainer,
> 
> *** Reporter, please consider answering these questions, where appropriate ***
> 
>    * What led up to the situation?
> 
>      Using unixodbc + freetds as a backend to pyodbc with MS-SQL server under
>      high load (many processes and threads using libodbc1 simultaneously).
> 
>      The file /etc/odbc.ini get randomly truncated, losing all connection data
>      and rendering all programs using unixodbc useless until we restore
> odbc.ini
>      from a backup.
> 
>    * What exactly did you do (or not do) that was effective (or
>      ineffective)?
> 
>      No triggering action by the user or programs was found. The file is
> truncated
>      seemingly at random. Many hours of high load may pass without problems.
> Under
>      low load, the problem was never found.
> 
>    * What was the outcome of this action?
> 
>      Does not apply here.
> 
>    * What outcome did you expect instead?
> 
>      Would be fine if the file odbc.ini would just stay put.
> 
>    * Countermeasure we tried
> 
>      A watchdog program was installed which notices the truncation and 
> replaces
> the
>      file. Also, auditd was installed to track changes to odbc.ini, which
> confirmed
>      libodbc1 as the culprit.
> 
>      This alleviates the situation, but does not solve the problem. During the
> short
>      time until the watchdog notices the destruction, the other clients are
> still
>      unusable.
> 
> 
> 
> -- System Information:
> Debian Release: buster/sid
>   APT prefers testing
>   APT policy: (500, 'testing'), (500, 'stable')
> Architecture: amd64 (x86_64)
> Foreign Architectures: i386
> 
> Kernel: Linux 4.13.0-1-amd64 (SMP w/4 CPU cores)
> Locale: LANG=en_US.UTF-8, LC_CTYPE=en_US.UTF-8 (charmap=UTF-8), 
> LANGUAGE=en_US.UTF-8 (charmap=UTF-8)
> Shell: /bin/sh linked to /bin/dash
> Init: systemd (via /run/systemd/system)

> --- unixodbc-2.3.4.orig/odbcinst/_odbcinst_SystemINI.c
> +++ unixodbc-2.3.4/odbcinst/_odbcinst_SystemINI.c
> @@ -176,11 +176,13 @@
>                 else
>          {
>              /* does not exist so try creating it */
> -            hFile = uo_fopen( pszFileName, "w" );
> -            if ( hFile )
> -                uo_fclose( hFile );
> -            else
> -                return FALSE;
> +            // hFile = uo_fopen( pszFileName, "w" );
> +            // if ( hFile )
> +            //     uo_fclose( hFile );
> +            // else
> +            //     return FALSE;
> +            /* Does not exist. Do nothing. */
> +            return FALSE;
>          }
>         }
>  
> 


-- 
Steve Langasek                   Give me a lever long enough and a Free OS
Debian Developer                   to set it on, and I can move the world.
Ubuntu Developer                                    http://www.debian.org/
slanga...@ubuntu.com                                     vor...@debian.org

Attachment: signature.asc
Description: PGP signature

Reply via email to