Skip to content

Commit 902f8fd

Browse files
dyemanovdyemanov
authored
Slightly better checking for a valid database (#8450)
1 parent 25fb454 commit 902f8fd

File tree

6 files changed

+68
-34
lines changed

6 files changed

+68
-34
lines changed

‎src/jrd/jrd.cpp‎

Lines changed: 35 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -749,6 +749,33 @@ namespace
749749
}
750750
}
751751

752+
USHORT validatePageSize(SLONG pageSize)
753+
{
754+
USHORT actualPageSize = DEFAULT_PAGE_SIZE;
755+
756+
if (pageSize > 0)
757+
{
758+
for (SLONG size = MIN_PAGE_SIZE; size <= MAX_PAGE_SIZE; size <<= 1)
759+
{
760+
if (pageSize <= size)
761+
{
762+
pageSize = size;
763+
break;
764+
}
765+
}
766+
767+
if (pageSize > MAX_PAGE_SIZE)
768+
pageSize = MAX_PAGE_SIZE;
769+
770+
fb_assert(pageSize <= MAX_USHORT);
771+
actualPageSize = (USHORT) pageSize;
772+
}
773+
774+
fb_assert(actualPageSize % PAGE_SIZE_BASE == 0);
775+
fb_assert(actualPageSize >= MIN_PAGE_SIZE && actualPageSize <= MAX_PAGE_SIZE);
776+
777+
return actualPageSize;
778+
}
752779

753780
class DefaultCallback : public AutoIface<ICryptKeyCallbackImpl<DefaultCallback, CheckStatusWrapper> >
754781
{
@@ -3005,18 +3032,7 @@ JAttachment* JProvider::createDatabase(CheckStatusWrapper* user_status, const ch
30053032

30063033
attachment->att_client_charset = attachment->att_charset = options.dpb_interp;
30073034

3008-
if (options.dpb_page_size <= 0) {
3009-
options.dpb_page_size = DEFAULT_PAGE_SIZE;
3010-
}
3011-
3012-
SLONG page_size = MIN_PAGE_SIZE;
3013-
for (; page_size < MAX_PAGE_SIZE; page_size <<= 1)
3014-
{
3015-
if (options.dpb_page_size < page_size << 1)
3016-
break;
3017-
}
3018-
3019-
dbb->dbb_page_size = (page_size > MAX_PAGE_SIZE) ? MAX_PAGE_SIZE : page_size;
3035+
dbb->dbb_page_size = validatePageSize(options.dpb_page_size);
30203036

30213037
TRA_init(attachment);
30223038

@@ -7634,20 +7650,22 @@ static JAttachment* create_attachment(const PathName& alias_name,
76347650

76357651
static void check_single_maintenance(thread_db* tdbb)
76367652
{
7637-
Database* const dbb = tdbb->getDatabase();
7653+
const auto dbb = tdbb->getDatabase();
7654+
const auto attachment = tdbb->getAttachment();
76387655

76397656
const ULONG ioBlockSize = dbb->getIOBlockSize();
76407657
const ULONG headerSize = MAX(RAW_HEADER_SIZE, ioBlockSize);
76417658

76427659
HalfStaticArray<UCHAR, RAW_HEADER_SIZE + PAGE_ALIGNMENT> temp;
7643-
UCHAR* header_page_buffer = temp.getAlignedBuffer(headerSize, ioBlockSize);
7660+
UCHAR* const header_page_buffer = temp.getAlignedBuffer(headerSize, ioBlockSize);
76447661

7645-
Ods::header_page* const header_page = reinterpret_cast<Ods::header_page*>(header_page_buffer);
7662+
if (!PIO_header(tdbb, header_page_buffer, headerSize))
7663+
ERR_post(Arg::Gds(isc_bad_db_format) << Arg::Str(attachment->att_filename));
76467664

7647-
PIO_header(tdbb, header_page_buffer, headerSize);
7665+
const auto header_page = reinterpret_cast<Ods::header_page*>(header_page_buffer);
76487666

76497667
if (header_page->hdr_shutdown_mode == Ods::hdr_shutdown_single)
7650-
ERR_post(Arg::Gds(isc_shutdown) << Arg::Str(tdbb->getAttachment()->att_filename));
7668+
ERR_post(Arg::Gds(isc_shutdown) << Arg::Str(attachment->att_filename));
76517669
}
76527670

76537671

‎src/jrd/ods.h‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -214,6 +214,9 @@ inline constexpr ULONG FIRST_SCN_PAGE = 2;
214214

215215
// Page size limits
216216

217+
inline constexpr USHORT PAGE_SIZE_BASE = 1024; // Minimal page size ever supported,
218+
// common divisor for valid page sizes
219+
217220
inline constexpr USHORT MIN_PAGE_SIZE = 4096;
218221
inline constexpr USHORT MAX_PAGE_SIZE = 32768;
219222

‎src/jrd/os/pio_proto.h‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -44,7 +44,7 @@ void PIO_extend(Jrd::thread_db*, Jrd::jrd_file*, const ULONG, const USHORT);
4444
void PIO_flush(Jrd::thread_db*, Jrd::jrd_file*);
4545
void PIO_force_write(Jrd::jrd_file*, const bool);
4646
ULONG PIO_get_number_of_pages(const Jrd::jrd_file*, const USHORT);
47-
void PIO_header(Jrd::thread_db*, UCHAR*, int);
47+
bool PIO_header(Jrd::thread_db*, UCHAR*, unsigned);
4848
USHORT PIO_init_data(Jrd::thread_db*, Jrd::jrd_file*, Jrd::FbStatusVector*, ULONG, USHORT);
4949
Jrd::jrd_file* PIO_open(Jrd::thread_db*, const Firebird::PathName&,
5050
const Firebird::PathName&);

‎src/jrd/os/posix/unix.cpp‎

Lines changed: 12 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -507,7 +507,7 @@ error: Raw device support for your OS is missing. Fix it or turn off raw device
507507
}
508508

509509

510-
void PIO_header(thread_db* tdbb, UCHAR* address, int length)
510+
bool PIO_header(thread_db* tdbb, UCHAR* address, unsigned length)
511511
{
512512
/**************************************
513513
*
@@ -519,13 +519,13 @@ void PIO_header(thread_db* tdbb, UCHAR* address, int length)
519519
* Read the page header.
520520
*
521521
**************************************/
522-
Database* const dbb = tdbb->getDatabase();
522+
const auto dbb = tdbb->getDatabase();
523523

524-
int i;
524+
unsigned i;
525525
SINT64 bytes;
526526

527-
PageSpace* pageSpace = dbb->dbb_page_manager.findPageSpace(DB_PAGE_SPACE);
528-
jrd_file* file = pageSpace->file;
527+
PageSpace* const pageSpace = dbb->dbb_page_manager.findPageSpace(DB_PAGE_SPACE);
528+
jrd_file* const file = pageSpace->file;
529529

530530
if (file->fil_desc == -1)
531531
unix_error("PIO_header", file, isc_io_read_err);
@@ -537,7 +537,11 @@ void PIO_header(thread_db* tdbb, UCHAR* address, int length)
537537
if (bytes < 0 && !SYSCALL_INTERRUPTED(errno))
538538
unix_error("read", file, isc_io_read_err);
539539
if (bytes >= 0)
540-
block_size_error(file, bytes);
540+
{
541+
FbLocalStatus tempStatus;
542+
if (!block_size_error(file, bytes, &tempStatus))
543+
return false;
544+
}
541545
}
542546

543547
if (i == IO_RETRY)
@@ -558,6 +562,8 @@ void PIO_header(thread_db* tdbb, UCHAR* address, int length)
558562
unix_error("read_retry", file, isc_io_read_err);
559563
}
560564
}
565+
566+
return true;
561567
}
562568

563569
// we need a class here only to return memory on shutdown and avoid

‎src/jrd/os/win32/winnt.cpp‎

Lines changed: 9 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -329,7 +329,7 @@ void PIO_force_write(jrd_file* file, const bool forceWrite)
329329
}
330330

331331

332-
void PIO_header(thread_db* tdbb, UCHAR* address, int length)
332+
bool PIO_header(thread_db* tdbb, UCHAR* address, unsigned length)
333333
{
334334
/**************************************
335335
*
@@ -345,11 +345,11 @@ void PIO_header(thread_db* tdbb, UCHAR* address, int length)
345345
* callers should not rely on this behavior
346346
*
347347
**************************************/
348-
Database* const dbb = tdbb->getDatabase();
348+
const auto dbb = tdbb->getDatabase();
349349

350-
PageSpace* pageSpace = dbb->dbb_page_manager.findPageSpace(DB_PAGE_SPACE);
351-
jrd_file* file = pageSpace->file;
352-
HANDLE desc = file->fil_desc;
350+
PageSpace* const pageSpace = dbb->dbb_page_manager.findPageSpace(DB_PAGE_SPACE);
351+
jrd_file* const file = pageSpace->file;
352+
const HANDLE desc = file->fil_desc;
353353

354354
OVERLAPPED overlapped;
355355
memset(&overlapped, 0, sizeof(OVERLAPPED));
@@ -361,10 +361,12 @@ void PIO_header(thread_db* tdbb, UCHAR* address, int length)
361361
{
362362
if (GetLastError() == ERROR_IO_PENDING)
363363
ret = GetOverlappedResult(desc, &overlapped, &actual_length, TRUE);
364+
365+
if (!ret)
366+
nt_error("ReadFile", file, isc_io_read_err, NULL);
364367
}
365368

366-
if (!ret || (length != actual_length))
367-
nt_error("ReadFile", file, isc_io_read_err, NULL);
369+
return (length == actual_length);
368370
}
369371

370372
// we need a class here only to return memory on shutdown and avoid

‎src/jrd/pag.cpp‎

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1115,10 +1115,15 @@ void PAG_header_init(thread_db* tdbb)
11151115
HalfStaticArray<UCHAR, RAW_HEADER_SIZE + PAGE_ALIGNMENT> temp;
11161116
UCHAR* const temp_page = temp.getAlignedBuffer(headerSize, ioBlockSize);
11171117

1118-
PIO_header(tdbb, temp_page, headerSize);
1119-
const header_page* header = (header_page*) temp_page;
1118+
if (!PIO_header(tdbb, temp_page, headerSize))
1119+
ERR_post(Arg::Gds(isc_bad_db_format) << Arg::Str(attachment->att_filename));
1120+
1121+
const auto header = (header_page*) temp_page;
1122+
1123+
if (header->hdr_header.pag_type != pag_header || header->hdr_header.pag_pageno != HEADER_PAGE)
1124+
ERR_post(Arg::Gds(isc_bad_db_format) << Arg::Str(attachment->att_filename));
11201125

1121-
if (header->hdr_header.pag_type != pag_header)
1126+
if (header->hdr_page_size < PAGE_SIZE_BASE || header->hdr_page_size % PAGE_SIZE_BASE != 0)
11221127
ERR_post(Arg::Gds(isc_bad_db_format) << Arg::Str(attachment->att_filename));
11231128

11241129
const USHORT ods_version = header->hdr_ods_version & ~ODS_FIREBIRD_FLAG;

0 commit comments

Comments
 (0)