Bareos Bug Tracker
Bareos Bug Tracker

View Issue Details Jump to Notes ] Related Changesets ] Issue History ] Print ]
IDProjectCategoryView StatusDate SubmittedLast Update
0000233bareos-core[All Projects] storage daemonpublic2013-10-09 17:152015-03-25 19:18
Reporterbastian 
Assigned To 
PrioritynormalSeveritymajorReproducibilityhave not tried
StatusclosedResolutionfixed 
PlatformLinuxOSanyOS Version3
Product Version13.2.1 
Target Version13.2.3Fixed in Version13.2.3 
Summary0000233: Missing handling of "close" errors
DescriptionPlease note: I have not (yet) reproduced the problem with bareos, but encountered it in Bacula. The respective code is unchanged, however.

When backing up on a CIFS target (others?), a full (remote) file system is not detected until close() returns (returning -1 and setting errno). However, the return value is not verified, and the incorrect write operation is not detected. The final job result is "ok", but the volume files are small or empty.
Steps To ReproduceConfigure a file based backup storage on a CIFS mounted share. Fill the remote file system until only a small amount of space is left. Run backup on that storage that contains more than that amount of data.

Results in an "OK" job and a full disk; no error messages whatsoever.
Additional InformationIn src/stored/dev.c, line 1735 (current git master head), "d_close(m_fd)" is called without checking its return value and/or errno.

I'd expect the attached patch to fix that problem. However, This Is Completely Untested[tm] to the extent that I did not even try to compile it!!!111einself

Thx to the brain-dead original project I'm not even able to fix the problem there, as Kern decided not to provide access to dcr in DEVICE::close. Thanks that you do :) One step closer to migrating.
TagsNo tags attached.
bareos-master: impact
bareos-master: action
bareos-18.2: impact
bareos-18.2: action
bareos-17.2: impact
bareos-17.2: action
bareos-16.2: impact
bareos-16.2: action
bareos-15.2: impact
bareos-15.2: action
bareos-14.2: impact
bareos-14.2: action
bareos-13.2: impact
bareos-13.2: action
bareos-12.4: impact
bareos-12.4: action
Attached Filesdiff file icon handle-close-errors.diff [^] (543 bytes) 2013-10-09 17:15 [Show Content]

- Relationships

-  Notes
(0000694)
mvwieringen (developer)
2013-10-09 17:40

Yes indeed its a bug not to check the return code of close(). It can indeed
return ENOSPC (just checked). Your patch almost works. e.g. the dev-> can
go as we are in the device class so we can just use errmsg which is a member
of the class you also fill that already. Other then I hate using a variable
named stat (I think I mostly changed them to status as stat is also the name
of a function ...). I see some more close calls but have to look if we want
to fail the job there too but ignoring the close() return value is indeed
something you should worry about.

Given this is good "defensive" programming anyway I will push it a least into
master soon. And if we have given it some time to settle there we can
backport it. I will commit the patch under your name later tonight.
(0000695)
mvwieringen (developer)
2013-10-09 17:55

It might be we have to explicitly check for errno == ENOSPC then again the
return values from close are:

All from the Solaris man page but I think its the same on all UNIX like systems.

The close() function will fail if:

EBADF The fildes argument is not a valid file descriptor.
EINTR The close() function was interrupted by a signal.
ENOLINK The fildes argument is on a remote machine and
           the link to that machine is no longer active.
ENOSPC There was no free space remaining on the device
           containing the file.

The close() function may fail if:

EIO An I/O error occurred while reading from or writing
           to the file system.

If I look at the errors above I think only EBADF is something not so
fatal then again its a programming error. So lets see what this patch
brings in regression tonight.
(0000696)
bastian (reporter)
2013-10-09 17:58

Fix committed to bareos master branch with changesetid 1208.
(0000701)
bastian (reporter)
2013-10-14 19:19

Fix committed to bareos bareos-12.4 branch with changesetid 1211.
(0000702)
bastian (reporter)
2013-10-14 19:19

Fix committed to bareos bareos-13.2 branch with changesetid 1214.
(0001359)
mvwieringen (developer)
2015-03-25 16:51

Fix committed to bareos2015 bareos-14.2 branch with changesetid 5014.
(0001514)
joergs (administrator)
2015-03-25 19:18

Due to the reimport of the Github repository to bugs.bareos.org, the status of some tickets have been changed. These tickets will be closed again.
Sorry for the noise.

- Related Changesets
bareos: master d2296ef6
Timestamp: 2013-10-09 15:43:36
Author: bastian
Ported: N/A
Details ] Diff ]
Missing handling of "close" errors

When backing up on a CIFS target (others?), a full (remote) file system
is not detected until close() returns (returning -1 and setting errno).
However, the return value is not verified, and the incorrect write
operation is not detected. The final job result is "ok", but the volume
files are small or empty.

Fixes 0000233: Missing handling of "close" errors

Signed-off-by: Marco van Wieringen <marco.van.wieringen@bareos.com>
mod - src/stored/dev.c Diff ] File ]
bareos: bareos-12.4 ce6baf19
Timestamp: 2013-10-09 15:43:36
Author: bastian
Ported: N/A
Details ] Diff ]
Missing handling of "close" errors

When backing up on a CIFS target (others?), a full (remote) file system
is not detected until close() returns (returning -1 and setting errno).
However, the return value is not verified, and the incorrect write
operation is not detected. The final job result is "ok", but the volume
files are small or empty.

Fixes 0000233: Missing handling of "close" errors

Signed-off-by: Marco van Wieringen <marco.van.wieringen@bareos.com>
mod - src/stored/dev.c Diff ] File ]
bareos: bareos-13.2 b27b5445
Timestamp: 2013-10-09 15:43:36
Author: bastian
Ported: N/A
Details ] Diff ]
Missing handling of "close" errors

When backing up on a CIFS target (others?), a full (remote) file system
is not detected until close() returns (returning -1 and setting errno).
However, the return value is not verified, and the incorrect write
operation is not detected. The final job result is "ok", but the volume
files are small or empty.

Fixes 0000233: Missing handling of "close" errors

Signed-off-by: Marco van Wieringen <marco.van.wieringen@bareos.com>
mod - src/stored/dev.c Diff ] File ]
bareos2015: bareos-12.4 6ab55b34
Timestamp: 2013-10-09 17:43:36
Author: bastian
Committer: mvwieringen
Ported: N/A
Details ] Diff ]
Missing handling of "close" errors

When backing up on a CIFS target (others?), a full (remote) file system
is not detected until close() returns (returning -1 and setting errno).
However, the return value is not verified, and the incorrect write
operation is not detected. The final job result is "ok", but the volume
files are small or empty.

Fixes 0000233: Missing handling of "close" errors

Signed-off-by: Marco van Wieringen <marco.van.wieringen@bareos.com>
mod - src/stored/dev.c Diff ] File ]
bareos2015: bareos-13.2 56ea00de
Timestamp: 2013-10-09 17:43:36
Author: bastian
Committer: mvwieringen
Ported: N/A
Details ] Diff ]
Missing handling of "close" errors

When backing up on a CIFS target (others?), a full (remote) file system
is not detected until close() returns (returning -1 and setting errno).
However, the return value is not verified, and the incorrect write
operation is not detected. The final job result is "ok", but the volume
files are small or empty.

Fixes 0000233: Missing handling of "close" errors

Signed-off-by: Marco van Wieringen <marco.van.wieringen@bareos.com>
mod - src/stored/dev.c Diff ] File ]
bareos2015: bareos-14.2 d32de662
Timestamp: 2013-10-09 17:43:36
Author: bastian
Committer: mvwieringen
Ported: N/A
Details ] Diff ]
Missing handling of "close" errors

When backing up on a CIFS target (others?), a full (remote) file system
is not detected until close() returns (returning -1 and setting errno).
However, the return value is not verified, and the incorrect write
operation is not detected. The final job result is "ok", but the volume
files are small or empty.

Fixes 0000233: Missing handling of "close" errors

Signed-off-by: Marco van Wieringen <marco.van.wieringen@bareos.com>
mod - src/stored/dev.c Diff ] File ]

- Issue History
Date Modified Username Field Change
2013-10-09 17:15 bastian New Issue
2013-10-09 17:15 bastian File Added: handle-close-errors.diff
2013-10-09 17:40 mvwieringen Note Added: 0000694
2013-10-09 17:40 mvwieringen Assigned To => mvwieringen
2013-10-09 17:40 mvwieringen Status new => assigned
2013-10-09 17:55 mvwieringen Note Added: 0000695
2013-10-09 17:58 bastian Changeset attached => bareos master d2296ef6
2013-10-09 17:58 bastian Note Added: 0000696
2013-10-09 17:58 bastian Assigned To mvwieringen => bastian
2013-10-09 17:58 bastian Status assigned => resolved
2013-10-09 17:58 bastian Resolution open => fixed
2013-10-14 19:19 bastian Changeset attached => bareos bareos-12.4 ce6baf19
2013-10-14 19:19 bastian Note Added: 0000701
2013-10-14 19:19 bastian Changeset attached => bareos bareos-13.2 b27b5445
2013-10-14 19:19 bastian Note Added: 0000702
2014-05-16 17:37 mvwieringen adm Assigned To bastian =>
2014-05-16 17:37 mvwieringen adm Status resolved => closed
2014-05-16 17:37 mvwieringen adm Fixed in Version => 13.2.3
2014-05-16 17:37 mvwieringen adm Target Version => 13.2.3
2015-03-25 16:51 mvwieringen Changeset attached => bareos2015 bareos-12.4 6ab55b34
2015-03-25 16:51 mvwieringen Changeset attached => bareos2015 bareos-13.2 56ea00de
2015-03-25 16:51 mvwieringen Changeset attached => bareos2015 bareos-14.2 d32de662
2015-03-25 16:51 mvwieringen Note Added: 0001359
2015-03-25 16:51 mvwieringen Status closed => resolved
2015-03-25 19:18 joergs Note Added: 0001514
2015-03-25 19:18 joergs Status resolved => closed


Copyright © 2000 - 2018 MantisBT Team
Powered by Mantis Bugtracker