BBufferGroup: Check for duplicated buffer id

* This exclude some possible cheating cases.
  It impose to have only a buffer_id per thread,
  so instances will not be duplicated if not needed.

headers/private/media/SharedBufferList.h

@@ -27,8 +27,15 @@ public:
 			status_t					Lock();
 			status_t					Unlock();
 
+			status_t					AddBuffer(sem_id groupReclaimSem,
+											const buffer_clone_info& info,
+											BBuffer** buffer);
+
+			// Call AddBuffer and CheckID locked
 			status_t					AddBuffer(sem_id groupReclaimSem,
 											BBuffer* buffer);
+			status_t					CheckID(sem_id groupSem,
+											media_buffer_id id) const;
 
 			status_t					RequestBuffer(sem_id groupReclaimSem,
 											int32 buffersInGroup, size_t size,

src/kits/media/BufferGroup.cpp

@@ -150,32 +150,12 @@ BBufferGroup::AddBuffer(const buffer_clone_info& info, BBuffer** _buffer)
 	if (fInitError != B_OK)
 		return B_NO_INIT;
 
-	// TODO: we need to make sure that a media_buffer_id is only added
-	// once to each group
-
-	BBuffer* buffer = new(std::nothrow) BBuffer(info);
-	if (buffer == NULL)
-		return B_NO_MEMORY;
-
-	if (buffer->Data() == NULL) {
-		// BBuffer::Data() will return NULL if an error occured
-		ERROR("BBufferGroup: error while creating buffer\n");
-		delete buffer;
-		return B_ERROR;
-	}
-
-	status_t status = fBufferList->AddBuffer(fReclaimSem, buffer);
+	status_t status = fBufferList->AddBuffer(fReclaimSem, info, _buffer);
 	if (status != B_OK) {
 		ERROR("BBufferGroup: error when adding buffer\n");
-		delete buffer;
 		return status;
 	}
-
 	atomic_add(&fBufferCount, 1);
-
-	if (_buffer != NULL)
-		*_buffer = buffer;
-
 	return B_OK;
 }
 

src/kits/media/SharedBufferList.cpp

@@ -162,6 +162,46 @@ SharedBufferList::Unlock()
 }
 
 
+status_t
+SharedBufferList::AddBuffer(sem_id groupReclaimSem,
+	const buffer_clone_info& info, BBuffer** _buffer)
+{
+	status_t status = Lock();
+	if (status != B_OK)
+		return status;
+
+	// Check if the id exists
+	status = CheckID(groupReclaimSem, info.buffer);
+	if (status != B_OK) {
+		Unlock();
+		return status;
+	}
+	BBuffer* buffer = new(std::nothrow) BBuffer(info);
+	if (buffer == NULL) {
+		Unlock();
+		return B_NO_MEMORY;
+	}
+
+	if (buffer->Data() == NULL) {
+		// BBuffer::Data() will return NULL if an error occured
+		ERROR("BBufferGroup: error while creating buffer\n");
+		delete buffer;
+		Unlock();
+		return B_ERROR;
+	}
+
+	status = AddBuffer(groupReclaimSem, buffer);
+	if (status != B_OK) {
+		delete buffer;
+		Unlock();
+		return status;
+	} else if (_buffer != NULL)
+		*_buffer = buffer;
+
+	return Unlock();
+}
+
+
 status_t
 SharedBufferList::AddBuffer(sem_id groupReclaimSem, BBuffer* buffer)
 {
@@ -170,12 +210,7 @@ SharedBufferList::AddBuffer(sem_id groupReclaimSem, BBuffer* buffer)
 	if (buffer == NULL)
 		return B_BAD_VALUE;
 
-	status_t status = Lock();
-	if (status != B_OK)
-		return status;
-
 	if (fCount == kMaxBuffers) {
-		Unlock();
 		return B_MEDIA_TOO_MANY_BUFFERS;
 	}
 
@@ -185,11 +220,27 @@ SharedBufferList::AddBuffer(sem_id groupReclaimSem, BBuffer* buffer)
 	fInfos[fCount].reclaimed = true;
 	fCount++;
 
-	status = release_sem_etc(groupReclaimSem, 1, B_DO_NOT_RESCHEDULE);
-	if (status != B_OK)
-		return status;
+	return release_sem_etc(groupReclaimSem, 1, B_DO_NOT_RESCHEDULE);
+}
 
-	return Unlock();
+
+status_t
+SharedBufferList::CheckID(sem_id groupSem, media_buffer_id id) const
+{
+	CALLED();
+
+	if (id == 0)
+		return B_OK;
+	if (id < 0)
+		return B_BAD_VALUE;
+
+	for (int32 i = 0; i < fCount; i++) {
+		if (fInfos[i].id == id
+			&& fInfos[i].reclaim_sem == groupSem) {
+			return B_ERROR;
+		}
+	}
+	return B_OK;
 }