This issue was introduced via patch "pdu: introduce ISCSI_PDU_CORK_WHEN_SENT"
on June 13, 2014 (commit 99585b6996).
Valgrind reported this use-after-free as follows:
Invalid read of size 4
at 0x5267606: iscsi_write_to_socket (socket.c:721)
by 0x5267A72: iscsi_service (socket.c:823)
by 0x526827C: event_loop (sync.c:67)
by 0x52698A4: iscsi_compareandwrite_sync (sync.c:823)
by 0x408111: compareandwrite (iscsi-support.c:1752)
by 0x4139E2: test_compareandwrite_simple (test_compareandwrite_simple.c:88)
by 0x503D260: ??? (in /usr/lib64/libcunit.so.1.0.1)
by 0x503D578: ??? (in /usr/lib64/libcunit.so.1.0.1)
by 0x503D8B5: CU_run_all_tests (in /usr/lib64/libcunit.so.1.0.1)
by 0x4046C6: main (iscsi-test-cu.c:1241)
Address 0x639f258 is 8 bytes inside a block of size 256 free'd
at 0x4C291E7: free (vg_replace_malloc.c:473)
by 0x525321B: iscsi_free (init.c:68)
by 0x52532F0: iscsi_sfree (init.c:110)
by 0x5257AD9: iscsi_free_pdu (pdu.c:179)
by 0x5267601: iscsi_write_to_socket (socket.c:719)
by 0x5267A72: iscsi_service (socket.c:823)
by 0x526827C: event_loop (sync.c:67)
by 0x52698A4: iscsi_compareandwrite_sync (sync.c:823)
by 0x408111: compareandwrite (iscsi-support.c:1752)
by 0x4139E2: test_compareandwrite_simple (test_compareandwrite_simple.c:88)
by 0x503D260: ??? (in /usr/lib64/libcunit.so.1.0.1)
by 0x503D578: ??? (in /usr/lib64/libcunit.so.1.0.1)
Signed-off-by: Bart Van Assche <bvanassche@acm.org>
Cc: Peter Lieven <pl@kamp.de>
This commit fixes two related issues in the ld_iscsi example with
largefile support.
The first issue appears when building libiscsi against the glibc C
library, with the flags -D_LARGEFILE_SOURCE -D_LARGEFILE64_SOURCE
-D_FILE_OFFSET_BITS=64:
{standard input}: Assembler messages:
{standard input}:2774: Error: symbol `__fxstat64' is already defined
{standard input}:2850: Error: symbol `__lxstat64' is already defined
{standard input}:2938: Error: symbol `__xstat64' is already defined
This is due to the fact that when _FILE_OFFSET_BITS=64 is passed, the
*64() functions are defined by the C library as aliases to the
non-64*() functions, because those ones directly support large files
(i.e 64 bits).
The second issue appears when building libiscsi against the uClibc C
library, in a configuration that doesn't have largefile support. In
this case, the ld_iscsi that tries to use stat64 or some other *64()
functions cannot build, because such functions are not provided by the
C library. Of course, ld_iscsi does not need to wrap the *64()
functions in such a situation, because they do not exist.
This commit fixes those problems by enclosing the *64() related code
of ld_iscsi in the following compile time conditional:
This ensures that the *64() function handling is here only if
largefile support is available and enabled (_LARGEFILE_SOURCE64) and
if non-*64() functions are not already 64 bits capable (in which case
the 64*() functions are just aliases for the non-64*() ones).
See also
http://www.gnu.org/software/libc/manual/html_node/Feature-Test-Macros.html
for more details about the meaning of those macros.
Signed-off-by: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
The current test in configure.ac for CUnit uses AC_TRY_RUN, which
doesn't work in a cross-compilation context, because we can't run on
the build machine the binaries that are built for the target. In
addition, the current logic assumes CUnit is available when the
AC_TRY_RUN test cannot be used (e.g in a cross-compilation case).
Since we actually don't care about *running* but only about testing
whether a simple program include the CUnit header file and linking
against the cunit library works, simply use AC_TRY_LINK() instead of
AC_TRY_RUN().
Signed-off-by: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
When trying to compile libiscsi on a 32 bit Ubuntu 10.04 with gcc 4.4.3
errors like the following are produced:
test_get_lba_status_beyond_eol.c:45: error: integer constant is too
large for ‘long’ type
This is because we don't specify that we are explictly compiling to the
c99 standard and as such gcc defaults to gnu90 standard. This in turn
means the maximum default type of integer literals is unsigned long int
which not big enough to hold the literal on a 32 bit architecture.
Fix this by adding the ULL suffix to all large integer literals.
Signed-off-by: Sitsofe Wheeler <sitsofe@yahoo.com>
As far as I know SPC-4 does not allow unaligned unmap requests to
fail. The only phrase I found in SPC-4 about unaligned unmap
requests is the following: "An unmap request with a starting LBA
that is not optimal may result in unmap operations on fewer LBAs
than requested." Hence check that unaligned unmap requests succeed.
Signed-off-by: Bart Van Assche <bvanassche@acm.org>
From the SPC-4 paragraph about WRITE SAME(10): "The WRITE SAME (10)
command requests that the device server transfer a single logical
block from the Data-Out Buffer [ ... ]". Hence always pass a data
buffer when sending a WRITE SAME(10) command.
Set the NDOB bit in the WRITE SAME(16) command if no data out buffer
is present.
Signed-off-by: Bart Van Assche <bvanassche@acm.org>
Support for WRITE SAME with NUMBER OF LOGICAL BLOCKS = 0 is optional.
Hence set NUMBER OF LOGICAL BLOCKS to 0 if this value is supported
and set this parameter to 1 otherwise.
Signed-off-by: Bart Van Assche <bvanassche@acm.org>
Only perform the test with 65536 blocks on block devices with at
least that number of blocks. Fix the 65536 block test log message.
Generalize the WRITE SAME(10) 256-block test log messages. Fix
interpretation of the MAXIMUM UNMAP BLOCK DESCRIPTOR COUNT field.
A quote from SPC-4: "If there is no limit on the number of UNMAP
block descriptors contained in the parameter data, then the device
server shall set the MAXIMUM UNMAP BLOCK DESCRIPTOR COUNT field to
FFFF_FFFFh."
Signed-off-by: Bart Van Assche <bvanassche@acm.org>
After checking several platforms, we do need this change in order to build.
If CentOS<old-version> does not manage we need to figure out why and how to fix it.
This reverts commit b9cf7e8cd2.
according to SBC-2 the alloc length should be at least 5 when
evpd is set to zero.
Also don't leak task if SPC-3 or later.
Signed-off-by: Peter Lieven <pl@kamp.de>
the test became nearly useless when reverting most
of its contents in commit 7d96336.
This patch adds some useful stuff back. While
we can't check that unmapped blocks are deallocated
we still can check that the blocks surrounding the
unmapped area don't get dealloaced.
Signed-off-by: Peter Lieven <pl@kamp.de>
Although SPC-2 specifies that a reservation conflict should be
reported when TEST UNIT READY is received from another initiator than
the registered initiator, it is an established practice that TEST
UNIT READY is accepted in this case. Hence use the MODE SENSE command
to test the effect of the RESERVE command instead of TEST UNIT READY.
Signed-off-by: Bart Van Assche <bvanassche@acm.org>
From the SBC-4 section about the block limits VPD page: "A write
same non-zero (WSNZ) bit set to one indicates that the device
server does not support a value of zero in the NUMBER OF LOGICAL
BLOCKS field in the WRITE SAME command CDBs (see 5.45, 5.46, and
5.47). A WSNZ bit set to zero indicates that the device server may
or may not support a value of zero in the NUMBER OF LOGICAL BLOCKS
field of the WRITE SAME commands." Hence verify that WRITE SAME
with NUMBER OF LOGICAL BLOCKS set to zero fails if WSNZ = 0.
Signed-off-by: Bart Van Assche <bvanassche@acm.org>
RFC 3720 is not clear about whether a target should return SUCCESS
or CHECK CONDITION if SPDTL > EDTL. Hence accept both. See also
Fred Knight, Re: [Ips] Data Out residual overflow/underflow handling,
IETF mailing list archive, 21 September 2012
(http://www.ietf.org/mail-archive/web/ips/current/msg02756.html).
Signed-off-by: Bart Van Assche <bvanassche@acm.org>
We can not test that UNMAP will actually unmap anything even if
it returned status GOOD from the device.
Devices are allowed to "do nothing and return status GOOD" whenever they
want to.
Signed-off-by: Ronnie Sahlberg <ronniesahlberg@gmail.com>
Add a test for unmapping a single range of blocks and that
get_lba_status returns correct data.
Test unmapping a single block at offsets 0-255
Test unmapping 1-255 blocks at offset 0 and verify that
get_lba_status at LBA 0 returns a descriptor of 1-255 blocks.
get_lba_status at the last block of the unmapped range returns
a descriptor for a single block.
Signed-off-by: Ronnie Sahlberg <ronniesahlberg@gmail.com>
Since writing headers and payload in a single iov has never been
implementend and after thinking about it several times seems to
be very hairy I would like to revert this change since
the original implementation is in O(1) while the changed one
is in O(n). This results in a complexity of O(n^2) instead of
O(n) for the whole send operation.
This reverts commit 06eab264f6.
if the socket is corked we otherwise set POLLOUT and then
do not sent. Depending on the event loop implementation this
can result in a busy wait.
Signed-off-by: Peter Lieven <pl@kamp.de>
Dell Equallogic sans report an invalid LBA offset in the descriptor when
using 4k LUNs. Check the descriptor for consistency with the request.
Signed-off-by: Peter Lieven <pl@kamp.de>
The outqueue_current PDU might also be in waitpdu if it does not have
ISCSI_PDU_DELETE_WHEN_SENT. outqueue_current is freed after the
waitpdu list (for reconnect or defer_reconnect), or sometimes not
considered at all (for cancel), and this can cause a dangling pointer.
Keep outqueue_current up to date when a PDU is freed. A bit hacky,
but it avoids touching code all over the place.
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
