Skip to content

Commit 6c8a15d

Browse files
committed
Fixed buffer over-read at the boundary.
- Say we are using a lock size of 1024k, we send a request for 128k at 1008k. It won't trigger the download, because we already download 1024k at 0. So it would read off from the empty disk space! - This problem only occurs during the first time you download a file. During subsequent accesses, when you are only reading from the cache, this problem does not occur.
1 parent 9e3e474 commit 6c8a15d

2 files changed

Lines changed: 15 additions & 17 deletions

File tree

src/cache.c

Lines changed: 13 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -948,14 +948,16 @@ static void *Cache_bgdl(void *arg)
948948
}
949949

950950
long Cache_read(Cache *cf, char * const output_buf, const off_t len,
951-
const off_t offset)
951+
const off_t offset_start)
952952
{
953953
long send;
954-
off_t dl_offset = offset / cf->blksz * cf->blksz;
954+
955+
/* The offset of the segment to be downloaded */
956+
off_t dl_offset = (offset_start + len) / cf->blksz * cf->blksz;
955957

956958
/* ------------------ Check if the segment already exists ---------------*/
957-
if (Seg_exist(cf, offset)) {
958-
send = Data_read(cf, (uint8_t *) output_buf, len, offset);
959+
if (Seg_exist(cf, dl_offset)) {
960+
send = Data_read(cf, (uint8_t *) output_buf, len, offset_start);
959961
goto bgdl;
960962
} else {
961963
/* Wait for any other download thread to finish*/
@@ -964,10 +966,10 @@ long Cache_read(Cache *cf, char * const output_buf, const off_t len,
964966
pthread_self());
965967
#endif
966968
PTHREAD_MUTEX_LOCK(&cf->w_lock);
967-
if (Seg_exist(cf, offset)) {
969+
if (Seg_exist(cf, dl_offset)) {
968970
/* The segment now exists - it was downloaded by another
969971
* download thread. Send it off and unlock the I/O */
970-
send = Data_read(cf, (uint8_t *) output_buf, len, offset);
972+
send = Data_read(cf, (uint8_t *) output_buf, len, offset_start);
971973
#ifdef CACHE_LOCK_DEBUG
972974
fprintf(stderr, "Cache_read(): thread %lu: unlocking w_lock;\n",
973975
pthread_self());
@@ -983,13 +985,6 @@ long Cache_read(Cache *cf, char * const output_buf, const off_t len,
983985
fprintf(stderr, "Cache_read(): thread %lu: ", pthread_self());
984986
long recv = path_download(cf->path, (char *) recv_buf, cf->blksz,
985987
dl_offset);
986-
if (recv < len) {
987-
send = recv;
988-
} else {
989-
send = len;
990-
}
991-
memmove(output_buf, recv_buf + (offset - dl_offset), send);
992-
993988
/*
994989
* check if we have received enough data, write it to the disk
995990
*
@@ -1006,6 +1001,7 @@ long Cache_read(Cache *cf, char * const output_buf, const off_t len,
10061001
"Cache_read(): received %ld, possible network error.\n", recv);
10071002
}
10081003
free(recv_buf);
1004+
send = Data_read(cf, (uint8_t *) output_buf, len, offset_start);
10091005

10101006
#ifdef CACHE_LOCK_DEBUG
10111007
fprintf(stderr, "Cache_read(): thread %lu: unlocking w_lock;\n",
@@ -1015,7 +1011,9 @@ long Cache_read(Cache *cf, char * const output_buf, const off_t len,
10151011

10161012
/* -----------Download the next segment in background -------------------*/
10171013
bgdl:
1018-
if ( (cf->next_offset > dl_offset) &&
1014+
;
1015+
off_t next_offset = round_div(offset_start, cf->blksz) * cf->blksz;
1016+
if ( (next_offset > dl_offset) &&
10191017
!Seg_exist(cf, cf->next_offset) &&
10201018
cf->next_offset < cf->content_length ){
10211019
/* Stop the spawning of multiple background pthreads */
@@ -1024,7 +1022,7 @@ long Cache_read(Cache *cf, char * const output_buf, const off_t len,
10241022
fprintf(stderr, "Cache_read(): thread %lu: trylocked bgt_lock;\n",
10251023
pthread_self());
10261024
#endif
1027-
cf->next_offset = round_div(offset, cf->blksz) * cf->blksz;
1025+
cf->next_offset = next_offset;
10281026
if (pthread_create(&cf->bgt, NULL, Cache_bgdl, cf)) {
10291027
fprintf(stderr,
10301028
"Cache_read(): Error creating background download thread\n"

src/cache.h

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -136,10 +136,10 @@ void Cache_delete(const char *fn);
136136
* \param[in] cf the cache in-memory data structure
137137
* \param[out] output_buf the output buffer
138138
* \param[in] len the requested segment size
139-
* \param[in] offset the start of the segment
139+
* \param[in] offset_start the start of the segment
140140
* \return the length of the segment the cache system managed to obtain.
141141
* \note Called by fs_read(), verified to be working
142142
*/
143143
long Cache_read(Cache *cf, char * const output_buf, const off_t len,
144-
const off_t offset);
144+
const off_t offset_start);
145145
#endif

0 commit comments

Comments
 (0)